diff --git a/.semgrep/baseline-counts.json b/.semgrep/baseline-counts.json index fa4cf540..cc8fb564 100644 --- a/.semgrep/baseline-counts.json +++ b/.semgrep/baseline-counts.json @@ -139,8 +139,8 @@ "", "PROMOTED 2026-08-11: the three Supabase error-handling rules (-discarded-result, -read-without-error, -read-without-error-fan-in) all sat at 0 and moved to chokepoints.yml, where they gate at --error across the whole tree rather than only on findings new vs. the PR base. Their keys are deleted here in the same change, per the promotion rule in .semgrep/README.md. No site was fixed in this change -- the burn-downs that got them to 0 are the 2026-08-07 and 2026-08-08 entries above; this is only the gate upgrade those burn-downs earned and never collected. Fire-checked before promoting, same protocol as -cross-tenant and -global-table: a scratch fixture with one deliberate violation per rule plus a correct control for each produced exactly 3 findings (the violations) and 0 on the controls, then was reverted. What remains in this file is the unbounded-select ladder alone: -in-list 11, -org-scoped 65, -single-parent 16." ], - "measured_at": "2026-08-12", + "measured_at": "2026-08-14", "counts": { - "fieldstay-supabase-unbounded-select-org-scoped": 59 + "fieldstay-supabase-unbounded-select-org-scoped": 58 } } diff --git a/CLAUDE.md b/CLAUDE.md index f2038598..45949a28 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1321,6 +1321,18 @@ following them stops being a memory test. Five layers, checked in CI via `information_schema.columns.is_generated`, NOT against `supabase/schema_reference.sql`, which renders generated columns as plain `DEFAULT`s and is exactly why this class shipped twice. + - `absence-reconciliation` — every "delete/cancel/deactivate every local row + missing from the fetched list" site is registered with HOW it survives an + empty fetch, and no provider fetch returns `[]` from an error branch. + Empty is the degenerate input of reconcile-by-absence: it makes every row + absent. On 2026-07-18 one org's entire Hospitable crew roster was + deactivated at the same microsecond because `hospFetchTeammates` returned + `[]` on a non-ok response and the deactivation pass had no guard. The two + valid protections are NOT interchangeable — `fetch-fails-loud` where empty + is a legitimate steady state (no calendar blocks, no assignments; a guard + there would make the LAST one unclearable), `empty-set-guard` where empty + is implausible and the fetch can't be trusted. Carries a self-check that + the scan still fires. - `public-route-rate-limiting` — every prefix in `proxy.ts`'s `TOKEN_ROUTES` has a matching branch in `rateLimiterForPathname()`, and the two guessable-invite-token `BYPASS_ROUTES` entries diff --git a/app/(dashboard)/ops/ops-snapshot.tsx b/app/(dashboard)/ops/ops-snapshot.tsx index bff8d5a5..daf55289 100644 --- a/app/(dashboard)/ops/ops-snapshot.tsx +++ b/app/(dashboard)/ops/ops-snapshot.tsx @@ -17,17 +17,20 @@ const AVG_DRIVE_SPEED_MPH = 30 // ── Types ────────────────────────────────────────────────────── -interface TurnoverAssignment { +export interface TurnoverAssignment { id: string crew_member: { id: string; name: string } | { id: string; name: string }[] | null } -interface Turnover { +export interface OpsTurnover { id: string property_id: string checkout_datetime: string + /** SYNTHETIC when prev_booking_id is null. */ checkin_datetime: string window_minutes: number | null + /** Null on a standalone turnover: nothing is booked after this checkout. */ + prev_booking_id: string | null status: string priority: string notes: string | null @@ -185,7 +188,7 @@ function TurnoverCard({ turnover, propertyName, }: Readonly<{ - turnover: Turnover + turnover: OpsTurnover propertyName: string }>) { const assignments = unwrapJoinArray(turnover.turnover_assignments) @@ -224,12 +227,14 @@ function TurnoverCard({ {checkout.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' })} - {turnover.window_minutes && ( + {/* Only a REAL next booking makes this a window. A standalone + turnover carries the generator's placeholder checkout + 4h. */} + {turnover.prev_booking_id && turnover.window_minutes ? ( <> · {Math.floor(turnover.window_minutes / 60)}h window - )} + ) : null}
@@ -277,7 +282,7 @@ function DayAccordion({ }: Readonly<{ label: string isToday: boolean - turnovers: Turnover[] + turnovers: OpsTurnover[] propertyMap: Record crewTravel?: CrewTravelSummary[] defaultOpen: boolean @@ -386,7 +391,7 @@ function getDayLabel(day: string, todayDate: string): string { // ── Urgency sort ───────────────────────────────────────────────── -function urgencyRank(t: Turnover): number { +function urgencyRank(t: OpsTurnover): number { const unassigned = t.status === 'pending_assignment' const urgent = t.priority === 'urgent' || t.priority === 'high' if (unassigned && urgent) return 0 @@ -398,10 +403,10 @@ function urgencyRank(t: Turnover): number { // ── Crew travel summary ───────────────────────────────────────── function getCrewTravelSummaries( - turnovers: Turnover[], + turnovers: OpsTurnover[], propertyById: Record ): CrewTravelSummary[] { - const byCrew: Record = {} + const byCrew: Record = {} for (const t of turnovers) { const assignments = unwrapJoinArray(t.turnover_assignments) @@ -447,7 +452,7 @@ export function OpsSnapshot({ metrics, showPmsRevenueNudge = false, }: Readonly<{ - turnovers: Turnover[] + turnovers: OpsTurnover[] properties: Property[] openWorkOrders: WorkOrder[] lowStockItems: LowStockItem[] diff --git a/app/(dashboard)/ops/page.tsx b/app/(dashboard)/ops/page.tsx index 9449b991..360e7893 100644 --- a/app/(dashboard)/ops/page.tsx +++ b/app/(dashboard)/ops/page.tsx @@ -1,7 +1,8 @@ import { requireOrgMember } from '@/lib/auth' import { createServiceClient } from '@/lib/supabase/server' import { unwrapList, type PostgrestResult } from '@/lib/supabase/unwrap' -import { OpsSnapshot } from './ops-snapshot' +import { OpsSnapshot, type OpsTurnover } from './ops-snapshot' +import { fetchAllRows } from '@/lib/inngest/paginate' import { addDays, subDays, startOfDay, endOfDay } from 'date-fns' import type { Metadata } from 'next' @@ -53,19 +54,39 @@ export default async function OpsSnapshotPage() { monthBookingsRes, pmsConnectionsRes, ] = await Promise.all([ - supabase - .from('turnovers') - .select(` - id, property_id, checkout_datetime, checkin_datetime, - window_minutes, status, priority, notes, completed_at, started_at, - checklist_template_id, - turnover_assignments(id, crew_member_id, crew_member:crew_members(id, name)) - `) - .eq('org_id', membership.org_id) - .neq('status', 'cancelled') - .gte('checkout_datetime', rangeStart.toISOString()) - .lte('checkout_datetime', rangeEnd.toISOString()) - .order('checkout_datetime', { ascending: true }), + // DRAINED, not a single .select(), matching turnovers/page.tsx. + // + // The 31-day window looks like it bounds this, and for a small org it does + // — but the ceiling is properties x turnovers-per-property-per-month, and + // at the 50-property target a busy calendar clears PostgREST's + // max_rows = 1000 inside one window. A .limit() would not have helped: + // max_rows caps the response regardless of what the query asks for, so the + // page would silently lose the far end of its own date range and report + // KPIs over a partial month with a 200 and no signal. + // + // .order('id') is the load-bearing half. .range() is OFFSET pagination, so + // the sort must be TOTAL or consecutive pages answer different questions — + // and checkout_datetime is emphatically not unique, since a portfolio + // shares 10am checkouts across every property. Same reasoning as the + // header comment on turnovers/page.tsx. + fetchAllRows( + (from, to) => supabase + .from('turnovers') + .select(` + id, property_id, prev_booking_id, checkout_datetime, checkin_datetime, + window_minutes, status, priority, notes, completed_at, started_at, + checklist_template_id, + turnover_assignments(id, crew_member_id, crew_member:crew_members(id, name)) + `) + .eq('org_id', membership.org_id) + .neq('status', 'cancelled') + .gte('checkout_datetime', rangeStart.toISOString()) + .lte('checkout_datetime', rangeEnd.toISOString()) + .order('checkout_datetime', { ascending: true }) + .order('id') + .range(from, to), + { label: `turnovers(ops-snapshot)[org=${membership.org_id}]` }, + ), supabase .from('properties') @@ -109,7 +130,9 @@ export default async function OpsSnapshotPage() { // app/(dashboard)/ops/error.tsx — a real error state, not a dashboard of // reassuring zeroes. const ctx = { site: 'page.ops', orgId: membership.org_id } - const allTurnovers = unwrapList(turnoversRes, { ...ctx, extra: { query: 'turnovers' } }) + // fetchAllRows already logs, reports and throws on a failed page, so this + // one is a plain array rather than a PostgrestResult to unwrap. + const allTurnovers = turnoversRes const properties = unwrapList(propertiesRes, { ...ctx, extra: { query: 'properties' } }) const openWorkOrders = unwrapList(openWOsRes, { ...ctx, extra: { query: 'work_orders' } }) const lowStockItems = unwrapList(belowParRes as PostgrestResult, { ...ctx, extra: { query: 'below_par' } }) diff --git a/app/(dashboard)/turnovers/[id]/page.tsx b/app/(dashboard)/turnovers/[id]/page.tsx index eda46519..60f36d0b 100644 --- a/app/(dashboard)/turnovers/[id]/page.tsx +++ b/app/(dashboard)/turnovers/[id]/page.tsx @@ -21,7 +21,7 @@ export default async function TurnoverDetailPage({ params }: Props) { const { data: turnover, error: turnoverError } = await supabase .from('turnovers') .select(` - id, property_id, checkout_datetime, checkin_datetime, + id, property_id, prev_booking_id, checkout_datetime, checkin_datetime, window_minutes, status, priority, notes, completion_notes, completed_at, auto_generated, checklist_template_id, bookings!booking_id ( guest_name, checkin_date, checkout_date, source ), @@ -134,9 +134,13 @@ export default async function TurnoverDetailPage({ params }: Props) {

Checkout

{formatDateTime(turnover.checkout_datetime)}

+ {/* Standalone turnover (no outgoing booking): checkin_datetime is + the generator's invented checkout + 4h, not a real arrival. */}

Next Check-in

-

{formatDateTime(turnover.checkin_datetime)}

+

+ {turnover.prev_booking_id ? formatDateTime(turnover.checkin_datetime) : 'No next booking'} +

diff --git a/app/(dashboard)/turnovers/page.tsx b/app/(dashboard)/turnovers/page.tsx index 3d169266..e06119e9 100644 --- a/app/(dashboard)/turnovers/page.tsx +++ b/app/(dashboard)/turnovers/page.tsx @@ -9,7 +9,7 @@ import { reportError } from '@/lib/observability/report-error' export const metadata: Metadata = { title: 'Turnovers' } const TURNOVER_COLUMNS = ` - id, property_id, booking_id, checkout_datetime, checkin_datetime, + id, property_id, booking_id, prev_booking_id, checkout_datetime, checkin_datetime, window_minutes, status, priority, notes, completed_at, started_at, crew_duration_minutes, checklist_template_id, is_same_day_turnover, is_archived, diff --git a/app/(dashboard)/turnovers/turnover-board.tsx b/app/(dashboard)/turnovers/turnover-board.tsx index 2f91f4ab..7da44a10 100644 --- a/app/(dashboard)/turnovers/turnover-board.tsx +++ b/app/(dashboard)/turnovers/turnover-board.tsx @@ -71,7 +71,18 @@ interface Turnover { // `=== 'owner_stay'` check below is unaffected by the wider type. stay_type: string | null checkout_datetime: string + /** + * SYNTHETIC when prev_booking_id is null — see the render below. The + * generator's standalone pass invents checkout + 4h so the turnover has a + * working window; there is no guest arriving at that time. + */ checkin_datetime: string + /** + * The OUTGOING booking on a paired turnover, and null on a standalone one. + * That null is the only signal that checkin_datetime was invented rather + * than read off a real arrival. + */ + prev_booking_id: string | null window_minutes: number | null status: string priority: string @@ -346,6 +357,14 @@ function TurnoverCard({ const isOverdue = isPast(checkout) && turnover.status !== 'completed' && turnover.status !== 'in_progress' const windowMins = turnover.window_minutes ?? 0 const windowColor = windowUrgencyColor(windowMins) + + // A turnover with no OUTGOING booking recorded is a standalone: nobody is + // arriving after this checkout that we know of. generator.ts's standalone + // pass still stores a checkin_datetime of checkout + 4h so the row has a + // usable working window, but that time is INVENTED. Rendering it as "In: + // 2:00 PM — 4h" states an arrival that does not exist, and a PM reading it + // schedules crew against a deadline nothing imposed. + const hasRealCheckin = turnover.prev_booking_id !== null const urgencyTone = turnoverUrgencyTone(isOverdue, turnover.priority) const duration = formatDurationMinutes(turnover.crew_duration_minutes) @@ -437,20 +456,26 @@ function TurnoverCard({ {checkout.toLocaleDateString('en-US', { weekday: 'short', month: 'short', day: 'numeric' })}{' '} {checkout.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' })} - → - - In:{' '} - {checkin.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' })} - {checkin.toDateString() !== checkout.toDateString() && ( - - ({checkin.toLocaleDateString('en-US', { month: 'short', day: 'numeric' })}) + {hasRealCheckin ? ( + <> + → + + In:{' '} + {checkin.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' })} + {checkin.toDateString() !== checkout.toDateString() && ( + + ({checkin.toLocaleDateString('en-US', { month: 'short', day: 'numeric' })}) + + )} - )} - - - - {formatWindow(windowMins)} - + + + {formatWindow(windowMins)} + + + ) : ( + No next booking + )}
{/* Auto-assignment suggestion banner */} diff --git a/app/(dashboard)/turnovers/turnover-gantt.tsx b/app/(dashboard)/turnovers/turnover-gantt.tsx index 71683ee5..b3bb655c 100644 --- a/app/(dashboard)/turnovers/turnover-gantt.tsx +++ b/app/(dashboard)/turnovers/turnover-gantt.tsx @@ -10,8 +10,11 @@ interface Turnover { id: string property_id: string checkout_datetime: string + /** SYNTHETIC when prev_booking_id is null — see the bar render below. */ checkin_datetime: string window_minutes: number | null + /** Null on a standalone turnover: nothing is booked after this checkout. */ + prev_booking_id: string | null status: string priority: string } @@ -332,12 +335,27 @@ export function TurnoverGantt({ turnovers, properties, bookings }: Props) { if (colIdx < 0 || colIdx >= TOTAL_DAYS) return null - // Tight window detection + // Tight window detection. A standalone turnover + // (prev_booking_id null) has NO next booking — the + // generator stores checkout + 4h so the row has a working + // window, but printing "4h" on the bar reads as a + // four-hour deadline, and it is the tightest-looking bar + // on the chart precisely when there is no pressure at all. + const hasRealCheckin = turnover.prev_booking_id !== null const windowMinutes = turnover.window_minutes ?? 0 const windowMs = windowMinutes * 60_000 const checkinDT = new Date(turnover.checkin_datetime) const gapMs = checkinDT.getTime() - checkoutDT.getTime() - const isTight = gapMs > 0 && gapMs < windowMs + const isTight = hasRealCheckin && gapMs > 0 && gapMs < windowMs + const windowHours = Math.round(windowMinutes / 60 * 10) / 10 + + // Built here rather than inline: the tight-window prefix + // nests a second ternary inside the has-a-checkin one. + let barTitle = `${turnover.status} · no next booking` + if (hasRealCheckin) { + const prefix = isTight ? 'Tight window — ' : '' + barTitle = `${prefix}${turnover.status} · ${windowHours}h window` + } const colors = turnoverColors(turnover.status, isTight) const leftPx = colIdx * COL_W + 2 @@ -358,9 +376,12 @@ export function TurnoverGantt({ turnovers, properties, bookings }: Props) { color: colors.fg, border: `1px solid ${colors.border}`, }} - title={`${isTight ? 'Tight window — ' : ''}${turnover.status} · ${Math.round(windowMinutes / 60 * 10) / 10}h window`} + title={barTitle} > - {isTight ? : } {Math.round(windowMinutes / 60 * 10) / 10}h + {isTight + ? + : } + {' '}{hasRealCheckin ? `${windowHours}h` : '—'} ) })} diff --git a/app/crew/page.tsx b/app/crew/page.tsx index 0153b2dc..35e25774 100644 --- a/app/crew/page.tsx +++ b/app/crew/page.tsx @@ -24,8 +24,11 @@ type TurnoverRow = { status: string priority: string checkout_datetime: string + /** SYNTHETIC when prev_booking_id is null. */ checkin_datetime: string window_minutes: number | null + /** Null on a standalone turnover: nothing is booked after this checkout. */ + prev_booking_id: string | null property_id: string } @@ -99,13 +102,16 @@ function TurnoverCard({ t, property }: { t: TurnoverRow; property?: PropertyRow {checkout.toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' })} - {t.window_minutes && ( + {/* Only a REAL next booking makes this a window. On a standalone + turnover window_minutes is the generator's placeholder 4h, and + showing it tells a cleaner to hurry for nobody. */} + {t.prev_booking_id && t.window_minutes ? ( {Math.floor(t.window_minutes / 60)}h {t.window_minutes % 60 > 0 ? ` ${t.window_minutes % 60}m` : ''} - )} + ) : null} ) diff --git a/app/crew/turnovers/[id]/page.tsx b/app/crew/turnovers/[id]/page.tsx index bb5b9c19..a455e6f7 100644 --- a/app/crew/turnovers/[id]/page.tsx +++ b/app/crew/turnovers/[id]/page.tsx @@ -142,12 +142,12 @@ export default function CrewTurnoverPage() { > {turnover.priority} priority - {turnover.window_minutes && ( + {turnover.prev_booking_id && turnover.window_minutes ? ( {Math.floor(turnover.window_minutes / 60)}h {turnover.window_minutes % 60 > 0 ? ` ${turnover.window_minutes % 60}m` : ''} window - )} + ) : null}
@@ -157,10 +157,17 @@ export default function CrewTurnoverPage() { {formatPropertyDateTime(turnover.checkout_datetime, property?.timezone ?? 'America/Chicago')}
+ {/* No outgoing booking recorded means this is a standalone turnover: + nobody is arriving after this checkout that we know of. The + generator still stores checkout + 4h as checkin_datetime so the + row has a working window, but showing that as "Next In" tells a + cleaner to hurry for a guest who does not exist. */}
Next In - {formatPropertyDateTime(turnover.checkin_datetime, property?.timezone ?? 'America/Chicago')} + {turnover.prev_booking_id + ? formatPropertyDateTime(turnover.checkin_datetime, property?.timezone ?? 'America/Chicago') + : 'No next booking'}
diff --git a/lib/dexie/schema.ts b/lib/dexie/schema.ts index a1399e03..65465c32 100644 --- a/lib/dexie/schema.ts +++ b/lib/dexie/schema.ts @@ -10,7 +10,19 @@ export interface TurnoverRow { property_id: string org_id: string checkout_datetime: string + /** + * SYNTHETIC when prev_booking_id is null — the generator's standalone pass + * stores checkout + 4h so the turnover has a working window. No guest is + * arriving then. Never render it as an arrival without checking + * prev_booking_id first. + */ checkin_datetime: string + /** + * The outgoing booking on a paired turnover, null on a standalone one. The + * only signal that checkin_datetime above was invented. Non-indexed, so no + * Dexie version bump. + */ + prev_booking_id: string | null window_minutes: number status: string priority: string diff --git a/lib/dexie/sync/turnovers.ts b/lib/dexie/sync/turnovers.ts index fe7b324b..456007d3 100644 --- a/lib/dexie/sync/turnovers.ts +++ b/lib/dexie/sync/turnovers.ts @@ -39,7 +39,7 @@ import { scopeChanged, rememberScope } from './scope' import { reportError } from '@/lib/observability/report-error' const TURNOVER_COLUMNS = - 'id, property_id, org_id, checkout_datetime, checkin_datetime, window_minutes, status, priority, notes, ' + + 'id, property_id, org_id, prev_booking_id, checkout_datetime, checkin_datetime, window_minutes, status, priority, notes, ' + 'inventory_started_at, inventory_confirmed_complete_at, inventory_confirmed_by_crew_id, completion_notes, ' + 'pending_checkout_datetime, pending_checkin_datetime, dates_changed_at, dates_change_acknowledged_at, updated_at' diff --git a/lib/inngest/functions/hospitable/teammate-sync-handler.ts b/lib/inngest/functions/hospitable/teammate-sync-handler.ts index 7e3b03bf..cb927d8b 100644 --- a/lib/inngest/functions/hospitable/teammate-sync-handler.ts +++ b/lib/inngest/functions/hospitable/teammate-sync-handler.ts @@ -21,6 +21,7 @@ import { createServiceClient } from '@/lib/supabase/server' import { getValidHospitableToken } from '@/lib/integrations/providers/hospitable-token' import { hospFetchTeammates, hospitableTeammatesToCrewRows } from '@/lib/integrations/providers/hospitable' import { logAuditEvents } from '@/lib/audit' +import { reportError } from '@/lib/observability/report-error' const PROVIDER = 'hospitable' @@ -67,6 +68,37 @@ export const hospTeammateSyncHandler = inngest.createFunction( const supabase = createServiceClient({ system: 'inngest:teammate-sync-handler' }) const freshExternalIds = new Set(teammates.map((t) => t.id)) + // An EMPTY fresh set is not "every teammate was removed upstream". + // + // This step reconciles by absence, which is the only way a removal is + // ever detectable — but absence-as-signal has one degenerate input, and + // hospFetchTeammates hands it to us readily: it returns [] on ANY non-ok + // response, including the 403 its own doc comment names as expected for a + // connection lacking the teammate:read scope. With an empty set every + // active Hospitable crew member is absent, so this deactivated the org's + // ENTIRE roster and wrote an audit row for each saying they were removed + // from Hospitable. + // + // That is not hypothetical. In production on 2026-07-18 at 09:00 UTC all + // three of one org's Hospitable crew members were deactivated at the same + // microsecond — one batch, the whole roster, from a single cron run. + // + // The asymmetry decides it, exactly as in ownerrez/reconciliation-handler + // and ical-sync: declining to deactivate leaves a stale crew row for one + // more day, while deactivating wrongly removes real people from + // scheduling and assignment. + if (freshExternalIds.size === 0) { + logger.error( + `[Hospitable teammate-sync] org ${org_id}: ZERO teammates returned — ` + + `skipping the deactivation pass rather than deactivating every crew member` + ) + reportError(new Error('Hospitable teammate sync returned zero teammates'), { + site: 'inngest.hospitable-teammate-sync-handler.empty-result-guard', + orgId: org_id, + }) + return 0 + } + const { data: existingActive, error: fetchErr } = await supabase .from('crew_members') .select('id, external_id') diff --git a/lib/inngest/functions/ownerrez/incremental-sync.ts b/lib/inngest/functions/ownerrez/incremental-sync.ts index 1c99a95e..59cab4cf 100644 --- a/lib/inngest/functions/ownerrez/incremental-sync.ts +++ b/lib/inngest/functions/ownerrez/incremental-sync.ts @@ -57,6 +57,7 @@ import { planBackfillWindow, advanceBackfill, readBackfillState, + revenuePostingFloor, } from '@/lib/integrations/providers/ownerrez-backfill' import { getRedis, upstashConfigured } from '@/lib/redis' import { RateLimitError, TokenRevokedError, translateSyncError } from '@/lib/integrations/types' @@ -409,7 +410,12 @@ async function persistBookings( return { affectedPropertyIds: Array.from(new Set(bookingRows.map((b) => b.property_id))), - bookingsToPostRevenue: selectOwnerRezBookingsToPostRevenue(bookingRows, idByExternalId), + // Revenue floor: only stays in the current month or later. A stay that + // predates FieldStay managing the property has no recoverable expense side, + // so posting its revenue alone overstates that month's net income. See + // revenuePostingFloor. + bookingsToPostRevenue: selectOwnerRezBookingsToPostRevenue( + bookingRows, idByExternalId, revenuePostingFloor(new Date())), // Blocks never generate turnovers (filtered at the generator query level), // but a known vacancy window is the best signal for scheduling maintenance. ownerBlocks: bookingRows.filter((r) => Boolean(r.is_block)), diff --git a/lib/inngest/functions/ownerrez/initial-sync.ts b/lib/inngest/functions/ownerrez/initial-sync.ts index 0d7f7b9a..37f293a8 100644 --- a/lib/inngest/functions/ownerrez/initial-sync.ts +++ b/lib/inngest/functions/ownerrez/initial-sync.ts @@ -23,7 +23,7 @@ import { selectOwnerRezBookingsToPostRevenue, } from '@/lib/integrations/providers/ownerrez' import { upsertBookingsReturningIds } from './upsert-bookings' -import { initialHistoryFrom } from '@/lib/integrations/providers/ownerrez-backfill' +import { initialHistoryFrom, revenuePostingFloor } from '@/lib/integrations/providers/ownerrez-backfill' import { logAuditEvent } from '@/lib/audit' import { applyMasterChecklistToProperty, @@ -639,7 +639,13 @@ export const ownerRezInitialSync = inngest.createFunction( bookingRows.map((b) => b.property_id).filter((id): id is string => id !== null) )) - bookingsToPostRevenue = selectOwnerRezBookingsToPostRevenue(bookingRows, idByExternalId) + // Revenue floor: current month onward only. The 90-day initial window + // has the same problem the backfill does — a stay that completed + // before the account connected has no cleaning fee, work order or + // restock recorded against it, because none of those happened in + // FieldStay. Its revenue alone is not a P&L, it is an overstatement. + bookingsToPostRevenue = selectOwnerRezBookingsToPostRevenue( + bookingRows, idByExternalId, revenuePostingFloor(new Date())) } try { diff --git a/lib/integrations/providers/hospitable.ts b/lib/integrations/providers/hospitable.ts index 78284961..7fdbacdf 100644 --- a/lib/integrations/providers/hospitable.ts +++ b/lib/integrations/providers/hospitable.ts @@ -511,8 +511,15 @@ export async function hospFetchProperties(token: string): Promise MAX_PAGES) { - console.error(`[Hospitable] properties pagination exceeded ${MAX_PAGES} pages — aborting`) - break + // THROW rather than break. `break` returned the pages gathered so far as + // if they were the complete set, and callers upsert that list and treat + // anything missing from it as gone — the same silent-truncation class as + // the OwnerRez pager. A loud failure is strictly better than a quietly + // short portfolio. + throw new Error( + `[Hospitable] properties pagination exceeded ${MAX_PAGES} pages ` + + `(${properties.length} so far) — refusing to return a partial result` + ) } const res = await hospitableFetch(url, token) @@ -631,8 +638,10 @@ export async function fetchReservationsWindow( while (page <= lastPage) { pageCount++ if (pageCount > MAX_PAGES) { - console.error(`[Hospitable] reservations pagination exceeded ${MAX_PAGES} pages — aborting`) - break + throw new Error( + `[Hospitable] reservations pagination exceeded ${MAX_PAGES} pages ` + + `(${reservations.length} so far) — refusing to return a partial result` + ) } // Build base params via URLSearchParams (handles encoding for all standard params) @@ -706,8 +715,10 @@ export async function hospFetchReviews( while (url) { pageCount++ if (pageCount > MAX_PAGES) { - console.error(`[Hospitable] reviews pagination exceeded ${MAX_PAGES} pages for property ${propertyId} — aborting`) - break + throw new Error( + `[Hospitable] reviews pagination exceeded ${MAX_PAGES} pages for property ${propertyId} ` + + `(${reviews.length} so far) — refusing to return a partial result` + ) } const res = await hospitableFetch(url, token) @@ -799,18 +810,37 @@ export async function hospFetchTeammates(token: string): Promise MAX_PAGES) { - console.error('[Hospitable] teammates pagination exceeded limit — aborting') - break + throw new Error( + `[Hospitable] teammates pagination exceeded ${MAX_PAGES} pages ` + + `(${teammates.length} so far) — refusing to return a partial result` + ) } const res = await hospitableFetch(url, token) if (!res.ok) { const text = await res.text().catch(() => '') - console.warn( + + // 403 is the ONE expected non-ok: a connection predating the + // teammate:read scope. Nothing is retriable about it, and teammate sync + // is additive, so an empty list is the honest answer. Callers that + // reconcile by absence must still guard an empty set — see + // teammate-sync-handler's deactivation pass. + if (res.status === 403) { + console.warn( + `[Hospitable] GET /teammates forbidden (missing teammate:read scope): ${text.slice(0, 200)}` + ) + return [] + } + + // Everything else THROWS. This returned [] for any status at all, from + // inside the pagination loop — so a 500 on page two discarded page one + // as well and reported "0 teammates" as a successful sync. Combined with + // the caller's absence-based deactivation, that is how an org's entire + // Hospitable crew roster was deactivated in one run. + throw new Error( `[Hospitable] GET /teammates failed (${res.status}): ${text.slice(0, 200)}` ) - return [] } const data = await res.json() as HospitablePagedTeammates diff --git a/lib/integrations/providers/hostaway.ts b/lib/integrations/providers/hostaway.ts index 15e0ba45..b57851c4 100644 --- a/lib/integrations/providers/hostaway.ts +++ b/lib/integrations/providers/hostaway.ts @@ -161,8 +161,13 @@ export async function hostawayFetchListings( while (true) { pageCount++ if (pageCount > MAX_PAGES) { - console.error(`[Hostaway] listings pagination exceeded ${MAX_PAGES} pages — aborting`) - break + // THROW rather than break — see the note on hospFetchProperties. A + // partial listing set returned as complete is indistinguishable from a + // shrunken portfolio to everything downstream. + throw new Error( + `[Hostaway] listings pagination exceeded ${MAX_PAGES} pages ` + + `(${listings.length} so far) — refusing to return a partial result` + ) } const res = await fetch( @@ -206,8 +211,10 @@ export async function hostawayFetchReservations( while (true) { pageCount++ if (pageCount > MAX_PAGES) { - console.error(`[Hostaway] reservations pagination exceeded ${MAX_PAGES} pages — aborting`) - break + throw new Error( + `[Hostaway] reservations pagination exceeded ${MAX_PAGES} pages ` + + `(${reservations.length} so far) — refusing to return a partial result` + ) } const params = new URLSearchParams({ diff --git a/lib/integrations/providers/ownerrez-backfill.ts b/lib/integrations/providers/ownerrez-backfill.ts index 570735dc..f3438bbb 100644 --- a/lib/integrations/providers/ownerrez-backfill.ts +++ b/lib/integrations/providers/ownerrez-backfill.ts @@ -69,6 +69,25 @@ export interface BackfillWindow { to: string } +/** + * First day of the current month — the floor for posting booking revenue to + * owner_transactions. + * + * A stay that ended before FieldStay was managing the property has no + * recoverable expense side: cleaning_fee posts on turnover completion, + * wo_completion on a work order, inventory_purchase on a received PO, and none + * of those exist for work done in another system. Posting its revenue anyway + * produces a month showing full rent against zero costs — an owner-facing P&L + * that is not merely incomplete but overstated. + * + * The current month is the boundary rather than the connection date because it + * is the one an operator can state plainly: "your FieldStay ledger starts this + * month." Anything earlier is visibly absent rather than quietly wrong. + */ +export function revenuePostingFloor(now: Date): string { + return `${now.toISOString().slice(0, 7)}-01` +} + /** Calendar date in UTC, as OwnerRez's date-only bounds expect. */ export function isoDate(d: Date): string { return d.toISOString().slice(0, 10) diff --git a/lib/integrations/providers/ownerrez.ts b/lib/integrations/providers/ownerrez.ts index 9ce26f1a..d5c39bfd 100644 --- a/lib/integrations/providers/ownerrez.ts +++ b/lib/integrations/providers/ownerrez.ts @@ -642,10 +642,34 @@ export function partitionMappedBookingRows(rows: OwnerRezBookingRow[]): { */ export function selectOwnerRezBookingsToPostRevenue( rows: OwnerRezBookingRow[], - idByExternalId: Record + idByExternalId: Record, + /** + * Earliest check-in date eligible for revenue posting (YYYY-MM-DD), or null + * for no floor. + * + * Revenue for a stay that predates FieldStay managing the property is + * REVENUE WITHOUT ITS EXPENSES, and that is worse than no data. All three + * expense sources on owner_transactions post when something COMPLETES inside + * FieldStay — cleaning_fee on turnover completion, wo_completion on a work + * order, inventory_purchase on a received PO — and none of those exist for a + * stay that happened before the account connected. Those jobs were done on + * paper or in another system and cannot be reconstructed. + * + * So a backfilled month would show full rent and zero costs: an inflated net + * income presented to a property owner as their P&L. A missing month is + * visibly missing; a wrong month is not. + * + * The bookings themselves are still imported across the whole backfill — + * they feed stay-length derivation for the par engine and occupancy history, + * neither of which is distorted by the absent expense side. + */ + minCheckinDate: string | null = null, ): { bookingId: string; propertyId: string; actualTotalAmount: number | null }[] { return rows .filter((b) => b.status === 'confirmed' && b.stay_type === 'guest_stay' && b.property_id !== null) + // ISO dates compare correctly as strings: fixed width, zero padded, + // most-significant first. + .filter((b) => minCheckinDate === null || (b.checkin_date ?? '') >= minCheckinDate) .map((b) => ({ bookingId: idByExternalId[b.external_id], propertyId: b.property_id as string, diff --git a/unit/guardrails/absence-reconciliation.test.ts b/unit/guardrails/absence-reconciliation.test.ts new file mode 100644 index 00000000..a2e3d849 --- /dev/null +++ b/unit/guardrails/absence-reconciliation.test.ts @@ -0,0 +1,259 @@ +import { describe, it, expect } from 'vitest' +import { readFileSync } from 'node:fs' +import { execSync } from 'node:child_process' + +// ============================================================================ +// RECONCILING BY ABSENCE — the destructive-by-default sync shape. +// +// "Fetch the upstream list, then delete/cancel/deactivate every local row that +// is missing from it" is the only way an upstream HARD DELETE is ever +// detectable, so this codebase uses it in five places. It also has one +// degenerate input that turns it into a data-destruction bug: an EMPTY fetched +// list means every local row is absent, so the pass removes all of them. +// +// This is not hypothetical. On 2026-07-18 at 09:00 UTC, hospTeammateSyncHandler +// deactivated all three of one org's Hospitable crew members at the same +// microsecond — one batch, the entire roster, one cron run — because +// hospFetchTeammates returned [] for a non-ok response and the deactivation +// pass had no empty-set guard. Every row got an audit entry claiming the person +// had been removed from Hospitable. +// +// THE INVARIANT: a fetch feeding an absence-based reconciliation must +// distinguish FAILURE from GENUINE EMPTINESS. There are exactly two valid ways, +// and which one is correct depends on whether empty is a legitimate steady +// state for that entity: +// +// fetch-fails-loud — the fetch throws (or returns null) on failure, so [] +// can only mean "upstream really has none". Required +// where empty is NORMAL: a property with no calendar +// blocks, a crew member with no assignments. An empty-set +// guard would be WRONG here — it would make the last +// block or last assignment impossible to ever clear. +// +// empty-set-guard — the pass refuses to act on an empty set at all. +// Required where empty is IMPLAUSIBLE and the fetch +// cannot be trusted to fail loudly. Costs one stale row +// until the next run; the alternative costs the table. +// +// Registering a site is deliberate. Adding a NEW absence-based reconciliation +// fails this test until someone states which protection it has and why. +// ============================================================================ + +const DESTRUCTIVE = /bulkDelete\(|\.delete\(\)|status:\s*'cancelled'|is_active:\s*false|deleted_at:/ +const ABSENCE = /!\s*(\w+)\s*\.has\(/ + +interface Reconciler { + protection: 'fetch-fails-loud' | 'empty-set-guard' + why: string +} + +/** + * Every absence-based reconciliation in the codebase, keyed by file:line. + * + * Shrink-only in spirit: a new entry needs a real justification, not a note + * that the check was noisy. + */ +const RECONCILERS: Record = { + 'lib/inngest/functions/ownerrez/reconciliation-handler.ts:163': { + protection: 'empty-set-guard', + why: 'Cancels FieldStay bookings absent from OwnerRez. An org with connected properties having zero bookings is implausible, and getBookings() can return a 200 with an empty body on an upstream hiccup — indistinguishable from a genuinely emptied account. Cancelling wrongly sends crew home from stays that are still happening.', + }, + 'lib/inngest/functions/hospitable/teammate-sync-handler.ts:112': { + protection: 'empty-set-guard', + why: 'Deactivates crew members absent from Hospitable. THE SITE THAT FIRED: hospFetchTeammates returned [] for any non-ok response, including the 403 expected for a connection without teammate:read, and did so from inside its pagination loop. Both halves are fixed, and the guard stays as the backstop because the caller cannot verify how the fetch failed.', + }, + 'lib/inngest/functions/hospitable/calendar-sync-handler.ts:122': { + protection: 'fetch-fails-loud', + why: 'Cancels blocks absent from the Hospitable calendar. hospFetchCalendar THROWS on any non-ok, so [] can only mean the window genuinely holds no blocks — which is the normal state for most properties. An empty-set guard here would be a bug: the LAST lifted block could never be cleared.', + }, + 'lib/dexie/sync/turnovers.ts:109': { + protection: 'fetch-fails-loud', + why: 'Drops cached turnovers no longer assigned to this crew member. fetchAssignedTurnoverIds returns NULL on failure and syncAssignedTurnovers returns early on null, so [] means the crew member genuinely has no assignments — a normal state, and unassignment-to-zero must still clear the device.', + }, + 'lib/dexie/sync/work-orders.ts:177': { + protection: 'fetch-fails-loud', + why: 'Drops cached work orders outside the crew member\'s current set. Both deltaPull and idSnapshot return NULL on failure and resolveDeltaPull propagates it, so [] means genuinely none assigned.', + }, +} + +/** + * A provider fetch that returns [] from an ERROR branch is the root cause the + * teammate wipe came from: it manufactures the degenerate input above, and no + * caller can tell the difference. Registered exceptions must be cases where + * the empty result is semantically true, not merely convenient. + */ +const FAIL_SOFT_EMPTY_RETURNS: Record = { + 'lib/integrations/providers/hospitable.ts:786': + 'hospFetchReservationMessages: 404 means the reservation has no message thread. Semantically empty, not a failure — and message sync is additive, never reconciled by absence.', + 'lib/integrations/providers/hospitable.ts:833': + 'hospFetchTeammates: 403 is the one expected non-ok — a connection predating the teammate:read scope. Nothing about it is retriable. Every OTHER status now throws, and the sole absence-based consumer carries an empty-set guard.', +} + +function sourceFiles(): string[] { + return execSync('grep -rl "\\.has(" --include=*.ts --include=*.tsx lib app', { encoding: 'utf8' }) + .trim().split('\n') + .filter((f) => f && !/\.test\.|\/stubs\//.test(f)) +} + +/** + * Finds every "absence drives a destructive write" site. + * + * `setName` is the Set being tested for membership — carried out of the scan so + * the empty-set-guard check can tie itself to THAT set rather than to any + * emptiness check anywhere in the file. A file-wide search was the first + * version and it was useless: teammate-sync-handler contains an unrelated + * `if (!rows.length) return 0` in its upsert step, so deleting the real guard + * still passed. + */ +function findReconcilerSites(): { site: string; setName: string }[] { + const found: { site: string; setName: string }[] = [] + + for (const file of sourceFiles()) { + const lines = readFileSync(file, 'utf8').split('\n') + + lines.forEach((line, i) => { + const absence = line.match(ABSENCE) + if (!absence) return + const setName = absence[1]! + + // What the absence check feeds: either an accumulator on the same line + // (`if (!set.has(x)) departed.add(x)`) or the const it is bound to. + let bind: string | null = null + const accumulator = line.match(/(\w+)\.add\(/) + if (accumulator) bind = accumulator[1]! + for (let b = i; !bind && b >= Math.max(0, i - 4); b--) { + const m = lines[b]!.match(/(?:const|let)\s+(\w+)\s*=/) + if (m) bind = m[1]! + } + if (!bind) return + + // A destructive write must follow, and must actually mention that value — + // otherwise this is an additive "create what's missing" filter, which is + // the common and harmless case. + const after = lines.slice(i, i + 45) + const body = after.join('\n') + if (!DESTRUCTIVE.test(body)) return + if (!new RegExp(`\\b${bind}\\b`).test(after.slice(1).join('\n'))) return + + found.push({ site: `${file}:${i + 1}`, setName }) + }) + } + + return found +} + +const findReconcilers = (): string[] => findReconcilerSites().map((r) => r.site) + +function findFailSoftEmptyReturns(): string[] { + const found: string[] = [] + const files = execSync('ls lib/integrations/providers/*.ts', { encoding: 'utf8' }) + .trim().split('\n').filter((f) => f && !/\.test\./.test(f)) + + for (const file of files) { + const lines = readFileSync(file, 'utf8').split('\n') + lines.forEach((line, i) => { + if (!/return \[\]/.test(line)) return + // Is this inside an error/status branch? Look back a few lines. + // 20 lines, not 8: an error branch can carry a long explanatory comment + // between the status check and the return, and an 8-line window silently + // missed exactly that shape when it was canaried. + const before = lines.slice(Math.max(0, i - 20), i + 1).join('\n') + if (!/!res\.ok|res\.status\s*===|catch\s*[({]/.test(before)) return + found.push(`${file}:${i + 1}`) + }) + } + return found +} + +describe('guardrail: reconciling by absence must survive an empty fetch', () => { + it('every absence-driven destructive write is registered with its protection', () => { + const unlisted = findReconcilers().filter((site) => !RECONCILES_KNOWN.has(site)) + + expect( + [ + unlisted.length === 0 ? '' : + 'A destructive write is driven by absence from a fetched set, and this site is not\n' + + 'registered. An EMPTY fetched list makes every local row absent, so this pass would\n' + + 'delete/cancel/deactivate all of them — that is exactly how one org\'s entire\n' + + 'Hospitable crew roster was deactivated in a single cron run on 2026-07-18.\n\n' + + 'Add it to RECONCILERS in this file, stating which protection it has:\n' + + ' fetch-fails-loud — the fetch throws/returns null on failure, so [] is genuinely empty\n' + + ' empty-set-guard — the pass refuses to act on an empty set\n\n' + + 'Unregistered sites:', + ...unlisted, + ].filter(Boolean).join('\n'), + ).toEqual('') + }) + + it('every registered site still exists at that file:line (prune when code moves)', () => { + const live = new Set(findReconcilers()) + const stale = Object.keys(RECONCILERS).filter((site) => !live.has(site)) + + expect( + [ + stale.length === 0 ? '' : + 'A RECONCILERS entry no longer matches any absence-driven destructive write.\n' + + 'Line numbers drift — re-point it, or delete the entry if the code is gone.\n' + + 'A stale entry silently stops protecting anything.\n\nStale entries:', + ...stale, + ].filter(Boolean).join('\n'), + ).toEqual('') + }) + + it('every empty-set-guard site actually contains an empty-set check', () => { + // The registry records intent; this checks the intent was implemented. + const bySite = new Map(findReconcilerSites().map((r) => [r.site, r.setName])) + + const missing = Object.entries(RECONCILERS) + .filter(([, r]) => r.protection === 'empty-set-guard') + .map(([site]) => site) + .filter((site) => { + const setName = bySite.get(site) + if (!setName) return false // staleness is the other test's job + const src = readFileSync(site.split(':')[0]!, 'utf8') + // The guard must name THE SET the absence check uses. + return !new RegExp(`${setName}\\.(size|length)\\s*===\\s*0|!${setName}\\.(size|length)\\b`).test(src) + }) + + expect( + [ + missing.length === 0 ? '' : + 'Registered as empty-set-guard, but the file has no empty-set check:', + ...missing, + ].filter(Boolean).join('\n'), + ).toEqual('') + }) + + it('every registration carries a real justification', () => { + for (const [site, r] of Object.entries(RECONCILERS)) { + expect(r.why.length, `${site} needs a real reason`).toBeGreaterThan(60) + } + }) + + it('no provider fetch returns [] from an error branch unless registered', () => { + const unlisted = findFailSoftEmptyReturns().filter((s) => !(s in FAIL_SOFT_EMPTY_RETURNS)) + + expect( + [ + unlisted.length === 0 ? '' : + 'A provider fetch returns [] from an error branch. That manufactures the degenerate\n' + + 'input the reconcilers above must survive, and no caller can tell it apart from a\n' + + 'genuinely empty upstream. Throw instead — or register it here if the empty result\n' + + 'is semantically TRUE rather than merely convenient.\n\nUnregistered:', + ...unlisted, + ].filter(Boolean).join('\n'), + ).toEqual('') + }) + + it('the scan itself fires — a broken checker looks exactly like a clean tree', () => { + // Self-check: the two shapes this test exists to catch must both be + // recognised. Without this, a regex typo turns the whole guardrail into a + // permanently-passing no-op. + const known = findReconcilers() + expect(known).toContain('lib/inngest/functions/hospitable/teammate-sync-handler.ts:112') + expect(known).toContain('lib/dexie/sync/work-orders.ts:177') // accumulator shape + expect(known.length).toBeGreaterThanOrEqual(5) + }) +}) + +const RECONCILES_KNOWN = new Set(Object.keys(RECONCILERS)) diff --git a/unit/inngest/hospitable-teammate-sync-handler.test.ts b/unit/inngest/hospitable-teammate-sync-handler.test.ts index 1d211d29..c92515e6 100644 --- a/unit/inngest/hospitable-teammate-sync-handler.test.ts +++ b/unit/inngest/hospitable-teammate-sync-handler.test.ts @@ -13,12 +13,16 @@ vi.mock('@/lib/integrations/providers/hospitable', () => ({ vi.mock('@/lib/audit', () => ({ logAuditEvents: vi.fn(), })) +vi.mock('@/lib/observability/report-error', () => ({ + reportError: vi.fn(), +})) import { hospTeammateSyncHandler } from '@/lib/inngest/functions/hospitable/teammate-sync-handler' import { createServiceClient } from '@/lib/supabase/server' import { getValidHospitableToken } from '@/lib/integrations/providers/hospitable-token' import { hospFetchTeammates, hospitableTeammatesToCrewRows } from '@/lib/integrations/providers/hospitable' import { logAuditEvents } from '@/lib/audit' +import { reportError } from '@/lib/observability/report-error' import { invokeHandler } from './test-helpers' function runAllStep() { @@ -182,3 +186,100 @@ describe('hospTeammateSyncHandler', () => { })).rejects.toThrow('Teammates upsert failed: db unavailable') }) }) + +// ========================================================================== +// The empty-fresh-set guard. +// +// This step reconciles by ABSENCE, and hospFetchTeammates used to return [] +// for ANY non-ok response — including the 403 its own doc comment names as +// expected for a connection without the teammate:read scope, and including a +// mid-pagination failure that also discarded the pages already gathered. +// +// With an empty set, every active Hospitable crew member is absent, so the +// deactivation pass removed the org's ENTIRE roster and wrote an audit row for +// each claiming they were removed from Hospitable. +// +// That happened. In production on 2026-07-18 at 09:00 UTC all three of one +// org's Hospitable crew members were deactivated at the SAME microsecond — +// one batch, the whole roster, from a single cron run. +// ========================================================================== +describe('hospTeammateSyncHandler — empty-result guard', () => { + beforeEach(() => { + vi.clearAllMocks() + ;(getValidHospitableToken as ReturnType).mockResolvedValue('token_abc') + }) + + it('deactivates NOBODY when Hospitable returns zero teammates', async () => { + ;(hospFetchTeammates as ReturnType).mockResolvedValue([]) + ;(hospitableTeammatesToCrewRows as ReturnType).mockReturnValue([]) + + const supabase = makeSupabase({ + crew_members: [ + // The read that would have supplied the whole roster as "removed". + { data: [ + { id: 'crew_1', external_id: 'tm_1' }, + { id: 'crew_2', external_id: 'tm_2' }, + { id: 'crew_3', external_id: 'tm_3' }, + ], error: null }, + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const result = await invokeHandler(hospTeammateSyncHandler, { + event: { data: EVENT_DATA }, + step: runAllStep(), + logger: makeLogger(), + }) + + expect(result).toEqual({ upserted: 0, deactivated: 0 }) + + // The two things that made this destructive rather than merely wrong. + const update = supabase.calls.find((c) => c.table === 'crew_members' && c.method === 'update') + expect(update).toBeUndefined() + expect(logAuditEvents).not.toHaveBeenCalled() + }) + + it('reports the empty result rather than passing it off as a clean run', async () => { + // Silence here is the whole problem: a wiped roster looked like a + // successful sync in the logs. + ;(hospFetchTeammates as ReturnType).mockResolvedValue([]) + ;(hospitableTeammatesToCrewRows as ReturnType).mockReturnValue([]) + const supabase = makeSupabase({ crew_members: [{ data: [], error: null }] }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const logger = makeLogger() + await invokeHandler(hospTeammateSyncHandler, { + event: { data: EVENT_DATA }, step: runAllStep(), logger, + }) + + expect(reportError).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ orgId: 'org_1' }), + ) + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('ZERO teammates')) + }) + + it('still deactivates normally when the fresh set is non-empty', async () => { + // The guard must not have disabled reconciliation altogether — a real + // removal still has to be detected. + ;(hospFetchTeammates as ReturnType).mockResolvedValue([{ id: 'tm_1' }]) + ;(hospitableTeammatesToCrewRows as ReturnType).mockReturnValue([ + { org_id: 'org_1', external_id: 'tm_1', external_source: 'hospitable' }, + ]) + const supabase = makeSupabase({ + crew_members: [ + { error: null }, + { data: [{ id: 'crew_gone', external_id: 'tm_old' }], error: null }, + { error: null }, + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const result = await invokeHandler(hospTeammateSyncHandler, { + event: { data: EVENT_DATA }, step: runAllStep(), logger: makeLogger(), + }) + + expect(result).toEqual({ upserted: 1, deactivated: 1 }) + }) +}) + diff --git a/unit/integrations/ownerrez-backfill.test.ts b/unit/integrations/ownerrez-backfill.test.ts index c760c588..4a0c11b5 100644 --- a/unit/integrations/ownerrez-backfill.test.ts +++ b/unit/integrations/ownerrez-backfill.test.ts @@ -5,6 +5,7 @@ import { advanceBackfill, initialHistoryFrom, readBackfillState, + revenuePostingFloor, isoDate, INITIAL_HISTORY_DAYS, BACKFILL_WINDOW_DAYS, @@ -177,3 +178,54 @@ describe('readBackfillState', () => { expect(readBackfillState({ backfill_complete: 1 }).complete).toBe(false) }) }) + +// ============================================================================ +// The revenue-posting floor. +// +// Backfilled bookings are imported across the whole two-year walk — they feed +// stay-length derivation and occupancy history. Their REVENUE is not posted, +// because the expense side cannot be reconstructed: cleaning_fee posts on +// turnover completion, wo_completion on a work order, inventory_purchase on a +// received PO, and none of those exist for work done before the account +// connected, on paper or in another system. +// +// A backfilled month would therefore show full rent against zero costs — an +// owner-facing P&L that is overstated rather than merely incomplete. A missing +// month is visibly missing; a wrong month is not. +// ============================================================================ +describe('revenuePostingFloor', () => { + it('is the first of the current month', () => { + expect(revenuePostingFloor(new Date('2026-08-13T12:00:00.000Z'))).toBe('2026-08-01') + }) + + it('holds on the first of the month — that day is inside the window, not before it', () => { + const floor = revenuePostingFloor(new Date('2026-08-01T00:00:00.000Z')) + expect(floor).toBe('2026-08-01') + expect('2026-08-01' >= floor).toBe(true) + }) + + it('holds on the last instant of a month without rolling forward', () => { + expect(revenuePostingFloor(new Date('2026-08-31T23:59:59.999Z'))).toBe('2026-08-01') + }) + + it('zero-pads single-digit months so string comparison stays chronological', () => { + // The floor is compared against checkin_date as a STRING. '2026-9-01' + // would sort after '2026-10-01' and silently admit the wrong bookings. + expect(revenuePostingFloor(new Date('2026-01-15T00:00:00.000Z'))).toBe('2026-01-01') + expect(revenuePostingFloor(new Date('2026-09-15T00:00:00.000Z'))).toBe('2026-09-01') + }) + + it('excludes every window the backfill walks', () => { + // The guarantee that matters: no window the historical walk produces can + // contain a stay eligible for revenue posting. + const now = new Date('2026-08-13T12:00:00.000Z') + const floor = revenuePostingFloor(now) + let state = { oldestCovered: null as string | null, complete: false } + for (let i = 0; i < 100; i++) { + const w = planBackfillWindow(state, now) + if (!w) break + expect(w.to < floor).toBe(true) + state = advanceBackfill(w, now) + } + }) +})