From 5e8df8b304b3eb14400dda9169f184e835411a77 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 19:07:57 +0000 Subject: [PATCH 1/6] Stop six paginators returning partial results as complete; fix the crew wipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six pagination loops ended a page-ceiling with `console.error` + `break`, handing the caller the pages gathered so far as if they were the whole set — the same silent-truncation class as the OwnerRez pager, one layer up. All six now throw: hospitable.ts properties / reservations / reviews / teammates hostaway.ts listings / reservations Chasing the teammates one surfaced something considerably worse, and it had already fired in production. hospTeammateSyncHandler's deactivate-removed-teammates step reconciles by ABSENCE — every active Hospitable crew member missing from the fetched list is set is_active = false, with an audit row saying "removed_from_hospitable". It had no empty-set guard, unlike ownerrez/reconciliation-handler and ical-sync, which both carry one for exactly this reason. And hospFetchTeammates fed it empty lists readily: it returned [] for ANY non-ok response — including the 403 its own doc comment names as expected for a connection lacking the teammate:read scope — from INSIDE the pagination loop, so a 500 on page two also discarded page one and reported a successful sync of zero teammates. Production, 2026-07-18 09:00 UTC: all three of one org's Hospitable crew members deactivated at the same microsecond. One batch, the entire roster, one cron run. The org has zero active Hospitable crew today. Both halves fixed: - The deactivation pass skips entirely on an empty fresh set, logs, and reports. Same asymmetry the other two guards cite — a stale crew row for one more day versus removing real people from scheduling and assignment. - hospFetchTeammates now throws on every non-ok EXCEPT 403, which alone is genuinely "no permission, nothing to sync" and stays non-fatal per the function's documented contract. That case is now safe because the caller guards it. Tests cover the guard directly: zero teammates must produce no UPDATE and no audit events, must report rather than pass as clean, and a non-empty set must still deactivate a genuinely removed teammate — the guard must not have turned reconciliation off. Canaried: removing the guard fails exactly the two tests that describe the wipe. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014Zk7eKd7UeNCoWxHSVxAFR --- .../hospitable/teammate-sync-handler.ts | 32 ++++++ lib/integrations/providers/hospitable.ts | 50 +++++++-- lib/integrations/providers/hostaway.ts | 15 ++- .../hospitable-teammate-sync-handler.test.ts | 101 ++++++++++++++++++ 4 files changed, 184 insertions(+), 14 deletions(-) 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/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/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 }) + }) +}) + From 130fe8b923e3b3b0b4ddc93709fff835cd97d994 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 21:21:04 +0000 Subject: [PATCH 2/6] Add the absence-reconciliation guardrail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reconciling by absence — "delete/cancel/deactivate every local row missing from the fetched list" — is the only way an upstream hard delete is ever detectable, and this codebase does it in five places. Its degenerate input is an EMPTY fetched list, which makes every local row absent. That is how one org's entire Hospitable crew roster was deactivated in a single cron run on 2026-07-18. Registering the five sites turned up something the "every reconciler needs an empty-set guard" rule I first reached for would have got WRONG. The two valid protections are not interchangeable: fetch-fails-loud — the fetch throws or returns null on failure, so [] can only mean upstream genuinely has none. Correct where empty is a normal steady state: a property with no calendar blocks, a crew member with no assignments. An empty-set guard here would be a bug — the LAST block or LAST assignment could never be cleared. empty-set-guard — refuse to act on an empty set. Correct where empty is implausible and the fetch cannot be trusted to fail loudly. Three of the five are already fetch-fails-loud and were never at risk: hospFetchCalendar throws on any non-ok, and both Dexie sync paths return NULL on failure and bail on it. That distinction — failure vs. genuine emptiness, made at the FETCH — is the real invariant, so the test also checks the root cause directly: no provider fetch may return [] from an error branch. Canaried three ways, and two of the three canaries failed against my first version, which is the only reason they are worth running: - A new unregistered reconciler in a scratch file — caught immediately. - Deleting the teammate empty-set guard — NOT caught at first. The check searched the whole file for any emptiness test, and that file has an unrelated `if (!rows.length) return 0` in its upsert step. Now the scan carries the Set's NAME out with the site and the guard must name that set. - Reverting hospFetchTeammates to fail-soft [] — NOT caught at first either. The error-branch lookback was 8 lines and the real branch carries a longer explanatory comment than that between the status check and the return. Widened to 20. Also carries a self-check asserting the scan still matches both known shapes (a filter binding and an `if (!set.has(x)) other.add(x)` accumulator), since a regex typo would turn the whole guardrail into a permanently-passing no-op that looks exactly like a clean tree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014Zk7eKd7UeNCoWxHSVxAFR --- CLAUDE.md | 12 + .../guardrails/absence-reconciliation.test.ts | 259 ++++++++++++++++++ 2 files changed, 271 insertions(+) create mode 100644 unit/guardrails/absence-reconciliation.test.ts 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/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)) From 079a1289e36bbcadd6829e1fa6571179f0a2bd55 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 21:56:41 +0000 Subject: [PATCH 3/6] Post booking revenue only from the current month, not across backfilled history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backfilled revenue lands in the right month — transaction_date is booking.checkin_date, not the post date — but landing in the right month is not the same as being right. Every expense on owner_transactions posts when something COMPLETES inside FieldStay: cleaning_fee on turnover completion, wo_completion on a work order, inventory_purchase on a received PO. None of those exist for a stay that happened before the account connected — that cleaning was paid for on paper or in another system, and it is not recoverable. The 45-day turnover history floor means no historical turnover is even created to complete. So a backfilled month would show full rent against zero costs, presented to a property owner as their P&L. That is overstated net income, not incomplete data, and a wrong month is worse than a missing one because nothing about it looks wrong. Revenue posting is therefore floored at the first of the current month, at BOTH OwnerRez call sites. The initial sync needed it as much as the backfill did: its 90-day window has exactly the same problem, since a stay that completed before the connection existed has no recorded costs either. The BOOKINGS are still imported across the whole two-year walk. They feed stay-length derivation for the par engine and occupancy history, neither of which the absent expense side distorts. Only the money is withheld. The floor is the first of the current month rather than the connection date because it is the boundary an operator can state plainly — "your FieldStay ledger starts this month" — and because it needs no per-connection lookup to evaluate. Tests cover the boundary conditions that a string comparison makes easy to get wrong: the first of the month is inside the window rather than before it, the last instant of a month does not roll forward, and single-digit months are zero-padded so '2026-09-01' does not sort after '2026-10-01'. Plus the property that ties it to the walk: no window planBackfillWindow produces can contain a stay eligible for posting. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014Zk7eKd7UeNCoWxHSVxAFR --- .../functions/ownerrez/incremental-sync.ts | 8 ++- .../functions/ownerrez/initial-sync.ts | 10 +++- .../providers/ownerrez-backfill.ts | 19 +++++++ lib/integrations/providers/ownerrez.ts | 26 +++++++++- unit/integrations/ownerrez-backfill.test.ts | 52 +++++++++++++++++++ 5 files changed, 111 insertions(+), 4 deletions(-) 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/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/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) + } + }) +}) From 42bdb12e6514ad1e673ddf5f494c3c083d39eb6c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 22:45:04 +0000 Subject: [PATCH 4/6] Stop showing an invented check-in time on standalone turnovers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A turnover with no NEXT booking still needs a working window, so generator.ts's standalone pass stores checkin_datetime = checkout + DEFAULT_STANDALONE_WINDOW_HOURS (4h) and window_minutes = 240. That is a placeholder, not an arrival. Every screen then rendered it as fact. A checkout at 10:00 AM with nobody booked after it displayed as "In: 2:00 PM — 4h", which reads as a hard deadline four hours out. It is the tightest-looking window in the list precisely when there is no pressure at all, so a PM schedules crew around a constraint nothing imposed and a cleaner rushes for a guest who does not exist. prev_booking_id is the discriminator, and it is exact rather than heuristic: the standalone insert writes prev_booking_id: null (generator.ts:357) and both pair paths write the outgoing booking id (:432, :508). Null therefore means the check-in time was invented. Fixed on the three surfaces a human reads as a deadline — the turnovers board, the PM turnover detail page, and the crew turnover detail page — all of which now show "No next booking" instead of a fabricated time and window. The board also drops the urgency-coloured window chip there, since colouring a placeholder by urgency is the same claim in another form. prev_booking_id had to be threaded to each: added to the board page's column list, the PM detail page's select, and — for crew — the Dexie TurnoverRow shape and TURNOVER_COLUMNS. It is non-indexed, so no Dexie version bump is required, per the note already in schema.ts covering the pending_* fields. The stored values are left alone. The 4h window is load-bearing elsewhere: turnover-created-events and crew-assignment both read window_minutes, and a null there would need its own handling in each. This commit makes the DISPLAY honest; whether a standalone turnover should carry a synthetic window at all is a separate question. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014Zk7eKd7UeNCoWxHSVxAFR --- app/(dashboard)/turnovers/[id]/page.tsx | 8 ++- app/(dashboard)/turnovers/page.tsx | 2 +- app/(dashboard)/turnovers/turnover-board.tsx | 51 +++++++++++++++----- app/crew/turnovers/[id]/page.tsx | 9 +++- lib/dexie/schema.ts | 12 +++++ lib/dexie/sync/turnovers.ts | 2 +- 6 files changed, 66 insertions(+), 18 deletions(-) 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/crew/turnovers/[id]/page.tsx b/app/crew/turnovers/[id]/page.tsx index bb5b9c19..1da3c525 100644 --- a/app/crew/turnovers/[id]/page.tsx +++ b/app/crew/turnovers/[id]/page.tsx @@ -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' From 8682bac6233d57de7882c34fb39453895a06d23c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 23:04:19 +0000 Subject: [PATCH 5/6] Finish the standalone-turnover display fix: Gantt, ops snapshot, crew MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 42bdb12e fixed the three surfaces that print a check-in TIME. Four more print the same placeholder as a WINDOW, which makes the identical claim in a different unit — and one of them is the Gantt bar that prompted this. turnover-gantt.tsx the bar's "4h" label and its tooltip ops/ops-snapshot.tsx "4h window" on the ops list crew/page.tsx the window chip on the crew's turnover list crew/turnovers/[id] the window chip beside the priority badge On a standalone turnover window_minutes is 240 because the generator invented a check-in at checkout + 4h, not because anything is due in four hours. The Gantt was the worst of the four: it renders that as the shortest bar label on the chart, so the turnover with no deadline at all looks like the most urgent thing on the page. All four now suppress the window when prev_booking_id is null — the Gantt shows an em dash with "no next booking" in its tooltip, the other three omit the chip entirely. The Gantt's isTight calculation is gated on the same condition, since comparing a synthetic gap against a synthetic window was never meaningful. prev_booking_id threaded to the two that lacked it: ops/page.tsx's select, and crew/page.tsx's local row type (the Dexie column was added in 42bdb12e). Also extracts the Gantt's tooltip string to a named variable rather than nesting a ternary inside a ternary, which sonarjs/no-nested-conditional flagged and which pushed the warning ceiling to 167 for one commit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014Zk7eKd7UeNCoWxHSVxAFR --- app/(dashboard)/ops/ops-snapshot.tsx | 9 ++++-- app/(dashboard)/ops/page.tsx | 2 +- app/(dashboard)/turnovers/turnover-gantt.tsx | 29 +++++++++++++++++--- app/crew/page.tsx | 10 +++++-- app/crew/turnovers/[id]/page.tsx | 4 +-- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/app/(dashboard)/ops/ops-snapshot.tsx b/app/(dashboard)/ops/ops-snapshot.tsx index bff8d5a5..3c495224 100644 --- a/app/(dashboard)/ops/ops-snapshot.tsx +++ b/app/(dashboard)/ops/ops-snapshot.tsx @@ -26,8 +26,11 @@ interface Turnover { 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 @@ -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}
diff --git a/app/(dashboard)/ops/page.tsx b/app/(dashboard)/ops/page.tsx index 9449b991..19cec4a4 100644 --- a/app/(dashboard)/ops/page.tsx +++ b/app/(dashboard)/ops/page.tsx @@ -56,7 +56,7 @@ export default async function OpsSnapshotPage() { 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, completed_at, started_at, checklist_template_id, turnover_assignments(id, crew_member_id, crew_member:crew_members(id, name)) 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 1da3c525..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}
From 050919b374312caf7fcb6e446d77b257c47986d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 01:18:50 +0000 Subject: [PATCH 6/6] Drain the ops-snapshot turnover read instead of taking one page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ratchet is gated on findings NEW versus the PR base, so adding prev_booking_id to this select re-presented a pre-existing unbounded read as a new one. It was worth fixing rather than re-baselining. The 31-day window (yesterday through +29 days) looks like it bounds this, and for a small org it does. The real 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 a single window. A .limit() would not have helped. max_rows caps the RESPONSE regardless of what the query asks for, so the page would have kept losing the far end of its own date range — ordered by checkout_datetime ascending, the rows dropped are the last days of the month — and reported KPIs over a partial window with a 200 and no truncation signal. That is the same class this rule exists for, and adding a limit would only have made the rule stop asking. So it drains through fetchAllRows, matching turnovers/page.tsx, which reads the same table over the same shape for the same reason. .order('id') is the load-bearing half of that change. .range() is OFFSET pagination, so the sort has to be TOTAL or consecutive pages answer different questions — and checkout_datetime is emphatically not unique when a portfolio shares 10am checkouts across every property. Without the tiebreaker, paginating would have been worse than not paginating. unwrapList is dropped for this read: fetchAllRows already logs, reports and throws on a failed page, so it hands back a plain array. Exports the row type from ops-snapshot as OpsTurnover so the page can name it — the type was previously inferred from the select, which fetchAllRows cannot do. Ratchet 59 -> 58, locked in per the tool's own instruction so the progress cannot silently regress. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014Zk7eKd7UeNCoWxHSVxAFR --- .semgrep/baseline-counts.json | 4 +-- app/(dashboard)/ops/ops-snapshot.tsx | 16 ++++----- app/(dashboard)/ops/page.tsx | 53 ++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 25 deletions(-) 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/app/(dashboard)/ops/ops-snapshot.tsx b/app/(dashboard)/ops/ops-snapshot.tsx index 3c495224..daf55289 100644 --- a/app/(dashboard)/ops/ops-snapshot.tsx +++ b/app/(dashboard)/ops/ops-snapshot.tsx @@ -17,12 +17,12 @@ 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 @@ -188,7 +188,7 @@ function TurnoverCard({ turnover, propertyName, }: Readonly<{ - turnover: Turnover + turnover: OpsTurnover propertyName: string }>) { const assignments = unwrapJoinArray(turnover.turnover_assignments) @@ -282,7 +282,7 @@ function DayAccordion({ }: Readonly<{ label: string isToday: boolean - turnovers: Turnover[] + turnovers: OpsTurnover[] propertyMap: Record crewTravel?: CrewTravelSummary[] defaultOpen: boolean @@ -391,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 @@ -403,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) @@ -452,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 19cec4a4..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, 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 }), + // 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' } })