Claude/field stay scalability lgfh15 - #508
Merged
Merged
Conversation
The notifications table (in-app bell events, added 2026-07-15) had no
retention job and grew forever — every other append-heavy table already
has one. New cron-notifications-retention runs daily at 9:30am CT
(15 min after dailyGuestPiiRetention, continuing the retention-cron
stagger) and enforces:
- read rows (read_at IS NOT NULL) older than 90 days: deleted
- all rows, read or unread, older than 180 days: deleted
Deletes run in bounded batches (select up to 500 ids, delete by id,
repeat, max 20 batches per policy per run) — never one unbounded
DELETE — so a large backlog degrades to "finish tomorrow" rather than
one giant transaction. Both steps are pure deletes and safe for Inngest
to replay. Service client created inside step.run() only, with the
{ system: 'inngest:notifications-retention' } context. Pure cron
trigger like the sibling retention crons — no new FieldStayEvents
entry needed. Log lines carry row counts only, never notification
content. Registered in the single serve() call in the Inngest route.
Unit test mirrors the queue-based .from() mock convention of
cron-comms-retention.test.ts: policy filters, bounded id-batch deletes,
batch-ceiling behavior, cutoff date math, and error-throw-for-retry.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Adds the database-side broadcast infrastructure for the crew PWA
realtime redesign (Option B: broadcast signal + delta pull), per
docs/CREW_SYNC_V2_PHASES.md section 2:
- notify_crew_sync(uuid[], text): shared realtime.send() helper on
topic 'crew:{user_id}', event 'sync', payload {entity} only (signal,
never row data). Per-user exception-safe so a broadcast failure can
never break the write that triggered it.
- Statement-level AFTER triggers with transition tables (one broadcast
per statement, not per row): turnover_assignments (INS/UPD/DEL) and
turnovers (UPD) -> 'turnovers'; checklist_instances and
checklist_instance_items (INS/UPD) -> 'checklists'; work_orders
(INS/UPD/DEL) -> 'work_orders'. UPDATE triggers notify old AND new
assignees so reassignment signals both sides.
- All functions SECURITY DEFINER with pinned empty search_path;
EXECUTE revoked from PUBLIC/anon/authenticated on the helper AND on
all five trigger functions (the latter added beyond the spec SQL:
Supabase security advisors flagged the trigger functions as
anon/authenticated-executable via PostgREST RPC, lints 0028/0029 --
confirmed cleared on re-run).
- RLS policy on realtime.messages authorizing each authenticated user
to receive broadcasts only on their own 'crew:{auth.uid()}' topic
(negative subscription test verified: foreign topic -> Unauthorized).
Applied to production (vpmznjktllhmmbfnxuvk) and e2e
(syhthijeqlnltufdawyb). Touch tests verified correct realtime.messages
rows for all three entities; scratch client received its own-topic
broadcast and was rejected from a foreign topic. No client subscribes
to these topics until Phase 3, so this ships with zero user-facing
behavior change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
…Phase 4) Failed outbox mutations previously re-pushed on every drain trigger (online event, every enqueueMutation), hammering the server during an outage. Now a push failure sets nextAttemptAt on the mutation row: Date.now() + min(2^(retryCount-1) * 5s, 5min) scaled by a uniform 0.5-1.5x jitter factor, so delays grow 5s -> 10s -> 20s ... capped at 5 minutes and a fleet of crew devices recovering from the same outage doesn't retry in lockstep. - lib/dexie/schema.ts: Dexie version(8); MutationRow gains nextAttemptAt?: number (epoch ms). Non-indexed - the drain scans in insertion order - so index strings are unchanged; the full store map is repeated as a complete snapshot per the file's v1-v3 pattern. - lib/dexie/syncService.ts processOutbox(): a head mutation still inside its backoff window stops the drain entirely (never skip-and-continue - later mutations against the same record must not jump ahead; "not due yet" joins the existing stop-on-first-error semantics) and schedules a single one-shot resume timer on the SyncEngine instance (previous timer cleared first). Success deletes the row, clearing nextAttemptAt with it. The MAX_RETRIES=5 dead-letter (failed: true) classification is preserved exactly. - unit/dexie/sync-outbox-backoff.test.ts (new, with additive fake-dexie.ts extensions: add/update/delete/orderBy on fakeTable, mutations table, update in the supabase chain): backoff growth, cap, and jitter bounds; drain stops at a not-yet-due head touching nothing behind it and resumes via the timer; due mutation retries and clears nextAttemptAt on success; permanent-failure path unchanged and excluded from later drains. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Adds the crew PWA's broadcast-signal client path per
docs/CREW_SYNC_V2_PHASES.md section 3, gated on
NEXT_PUBLIC_CREW_SYNC_V2 so it ships dormant — the flag defaults off
and the existing three postgres_changes channels + generation-token
machinery in lib/dexie/context.tsx are untouched (pure additions: one
early-return in run() when the flag is on, new teardown lines appended
after the existing ones, no modified lines in the v1 path).
Flag on: one private broadcast channel (crew:{userId}), subscribed
after supabase.realtime.setAuth() and re-authed on TOKEN_REFRESHED;
signals route through a per-entity 1s trailing debounce with in-flight
serialization (exactly one queued follow-up, never stacked); each
entity resolves to a full-scope pull so cursor advancement stays safe
per the doc's cursor invariants. Safety poll every 5 minutes (also the
only freshness path for property_assets, which has no trigger by
design) plus resyncs on mount/online/visibilitychange/(re)subscribe.
Reconnect on CHANNEL_ERROR/TIMED_OUT/CLOSED after 5s base + uniform
[0,30s] jitter to avoid a rejoin thundering herd.
The per-entity debouncer, entity->action mapping, and reconnect-jitter
math are extracted into lib/dexie/sync/signals.ts as pure/injectable
functions, unit-tested with fake timers in unit/dexie/sync-signals.test.ts
and unit/dexie/sync-reconnect-jitter.test.ts (burst coalescing,
in-flight serialization with a single queued follow-up, unknown
entities ignored, each entity invoking its own sync fn, jitter always
within [5s, 35s]). Documents NEXT_PUBLIC_CREW_SYNC_V2 in .env.example.
Deviation: the doc's Phase 1 field-soak precondition is waived per the
project owner — pre-launch, no field crew traffic exists yet to soak
against.
Manual smoke (doc 3d): the disposable e2e crew user/turnover were
seeded and torn down on the syhthijeqlnltufdawyb project, and the app
was built+started against that project with the flag on. The
browser-driven half of the smoke (Playwright/Chromium hitting
Supabase auth through the sandbox's required HTTPS proxy) could not be
completed — confirmed via a minimal repro that an unrelated bare
page.goto() to Supabase's own endpoint fails the same way through
Chromium's network stack over this proxy, while curl through the
identical proxy succeeds, ruling out a TLS/cert issue and pointing at
a sandbox/Chromium proxy-tunneling limitation rather than anything in
this change. Phase 2's already-completed pure-Node scratch-client test
independently proved the wire-level mechanism this phase's UI consumes
(private channel subscribe via realtime.setAuth(), broadcast delivery,
cross-user-topic rejection). Full verification pass (tsc, lint,
vitest, check:ui-classes) is green.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Closes the last item from PR #505's db-invariants job: check 4 (types vs. live schema) was deferred. Adds public.db_type_shape_report() (mirrors db_invariant_report()'s SECURITY DEFINER/service-role-only pattern) and scripts/check-type-drift.mjs, which diffs it against a mechanical parse of types/database.ts — every enum's labels, every table's presence, and column presence for every table wired into Database.public.Tables. Wired into the existing db-invariants CI job as a second step. First real run against both projects surfaced three genuine drift incidents beyond the wo_status one that motivated this check: - wo_source was missing 'vacancy_gap_suggestion', which advanceScheduleAfterCompletion() already branches on (20260725201000_add_vacancy_gap_suggestion_to_wo_source.sql). - crew_feedback's timestamp column was renamed created_at -> submitted_at on both projects by an earlier drift-capture migration, but support-inbox/page.tsx and its client still queried/displayed the old name — a live "column does not exist" bug in the support inbox feedback list, now fixed alongside the stale CrewFeedback interface. - inventory_count_drafts never actually had the reviewed_at/reviewed_by columns approveInventoryCount()/rejectInventoryCount() write to (an earlier migration defined them but no-op'd against an already-existing table) — every PM approve/reject of a pending count was failing. Added the columns for real, with their FK covering index (20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql). Also reconciled a long tail of types/database.ts fields that had drifted from the live schema (BookingSource.ownerrez, SupportMessageRole.human, Organization/Property/Booking/Turnover/WorkOrder/MaintenanceSchedule columns, TurnoverAssignment/PushSubscription/SupportConversation/ SupportMessage/InventoryCountDraft(Item)/InventoryTemplateItem shapes, new OrgSmsTemplate interface) and wired a dozen previously-unmapped but already-interfaced tables into Database.public.Tables. Intentional mismatches (the deprecated work_orders.assigned_crew_id, join-only relationship fields, DB-internal-only tables like platform_admins) are allowlisted in the new script, same shrink-only ratchet as SERVICE_ROLE_ONLY_TABLES. Migrations applied to both vpmznjktllhmmbfnxuvk and syhthijeqlnltufdawyb. get_advisors on production shows no new findings from db_type_shape_report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Product decision: Hostaway must not be reachable at all right now, but must stay easy to re-enable later. The connect UI/server-actions were already disabled in 4c239e3 (revenue-posting gap), but the two real chokepoints were still live — this closes them: - lib/integrations/registry.ts: comment out the hostawayProvider import and its map entry, so getProvider('hostaway') now throws (webhook route at app/api/webhooks/[provider]/route.ts already 404s on that, and disconnectIntegration's revoke path already no-ops on failure) - app/api/inngest/route.ts: comment out the hostawayInitialSync import and its serve() array entry, so the job can never be invoked by Inngest even if something (a stray manual trigger, a resurrected send()) tried to fire integration/hostaway.sync.requested Both lib/integrations/providers/hostaway.ts and lib/inngest/functions/hostaway/initial-sync.ts are left functionally untouched — only a top-of-file comment was added pointing at exactly what's commented out and where, so re-enabling is a matter of uncommenting four spots rather than reconstructing anything. Verified no test asserts on registry contents or the Inngest functions array (all mock the registry module wholesale), so the existing Hostaway-specific tests still validate the adapter/job in isolation unchanged. ops/page.tsx and lib/support/account-tools.ts's Hostaway mentions are pre-existing generic/example references unrelated to the connect flow — left alone so support tooling still surfaces any historical connection state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
… spend gate The daily nudge budget check in lib/sms/telnyx.ts already fails closed on a Redis error (shipped in 268beb5) — this only adds reportError alongside the existing console.error so the outage surfaces in Sentry, not just logs, and documents the fail-closed/fail-open split in CLAUDE.md's SMS section per docs/SCALABILITY_TIERS_REMAINING.md section 4. Audited every other Redis- backed limiter in the codebase (lib/rate-limit.ts consumers, proxy.ts, OwnerRez/Hospitable API budgets) — none of them gate real spend, so none needed a fail-open -> fail-closed change; see session report for the full classification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Cart automation fanning out across orgs shares one app-level Kroger client credential (one Vercel deployment IP/token), same class of problem as OwnerRez/Hospitable's shared-IP budgets — but Kroger had no limiter at all before this, so a busy cart-build tick could burn the whole platform's daily quota and 429 every other org silently. lib/rate-limit.ts gains four endpoint-class Upstash sliding-window limiters, each a fixed shared identifier (not per-org): products (9,000/day, 90% headroom off Kroger's confirmed 10,000/day) and locations (1,440/day, off the confirmed 1,600/day) are sourced from developer.kroger.com; cart and auth/identity limits aren't published there, so those two use clearly-commented conservative defaults. lib/kroger/client.ts routes every outbound call through a new krogerFetch() wrapper (same shape as hospitableFetch in providers/hospitable.ts): checks the relevant limiter first, reacts to a real 429 by parsing Retry-After, and fails open on a Redis error since this is an abuse/quota limiter, not a spend-budget one. Both throw the existing shared RateLimitError. Inside Inngest steps (build-shopping-cart.ts, kroger-connected.ts, integration-token-refresh-handler.ts) a RateLimitError now propagates so Inngest's own backoff retries the step — build-shopping-cart.ts's get-customer-token step previously swallowed every error, including a rate limit, into a silent list-only fallback; it now rethrows RateLimitError specifically before that fallback. The two OAuth callback routes run outside any Inngest step, so a rate limit there gets a distinct rate_limited reason on /connect/error instead of the generic token_exchange_failed/restart-connect-flow path, since neither route has a retry mechanism to lean on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Hostaway deliberately disabled (not built) per product decision; Kroger rate limiter, fail-closed spend budgets, and drift check all shipped. Only Crew Sync v2 Phase 5 remains open. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughCrew Sync v2 adds opt-in private Realtime broadcasts with debounced entity pulls, reconnect handling, safety resyncs, and teardown. The PR also adds schema drift CI checks, outbox retry backoff, Kroger rate limiting, notification retention, Hostaway disabling, UI/type alignment, and SMS failure reporting. ChangesSync, schema, and reliability changes
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Database
participant Realtime
participant DexieProvider
participant SyncSignalHandler
participant DeltaPull
Database->>Realtime: Broadcast entity signal
Realtime->>DexieProvider: Deliver private crew topic event
DexieProvider->>SyncSignalHandler: Route entity payload
SyncSignalHandler->>DeltaPull: Debounce and execute entity pull
DeltaPull->>DexieProvider: Update local Dexie state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
66-147: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNo
permissions:block — jobs inherit whatever default GITHUB_TOKEN scope the org/repo has set.Neither the workflow top level nor the
e2e/db-invariantsjobs declarepermissions:, so "these default permissions are excessively permissive; for example, they include read/write access to all issues, PRs, and repository contents" if the repo/org hasn't hardened its default. None of these jobs (build checks, Playwright e2e, DB invariant checks) need write access.+permissions: + contents: read + name: ... jobs: checks:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 66 - 147, Add an explicit least-privilege permissions declaration at the workflow level, setting GITHUB_TOKEN access to read-only repository contents and disabling unnecessary scopes. Ensure the build, e2e, and db-invariants jobs inherit these restricted permissions without granting write access to issues, pull requests, contents, or other resources.Source: Linters/SAST tools
🧹 Nitpick comments (3)
scripts/check-type-drift.mjs (1)
161-169: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the schema-report fetch.
No timeout is set on the RPC call; a hung connection would stall this CI step for the full job/runner timeout rather than failing fast.
🔧 Proposed fix
const res = await fetch(new URL('/rest/v1/rpc/db_type_shape_report', url), { method: 'POST', headers: { apikey: key, authorization: `Bearer ${key}`, 'content-type': 'application/json', }, body: '{}', + signal: AbortSignal.timeout(30_000), })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-type-drift.mjs` around lines 161 - 169, Configure a finite timeout for the RPC request in the schema-report fetch, using the existing fetch options around the db_type_shape_report call. Ensure a hung connection aborts promptly and causes the CI step to fail rather than waiting for the runner timeout.supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql (1)
19-27: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSplit into zero-downtime migrations to avoid blocking locks.
Adding the FK constraint here takes a table scan +
SHARE ROW EXCLUSIVElock, and the plainCREATE INDEXblocks writes during the build. Supabase applies each migration file inside a transaction, and "CONCURRENTLY cannot run inside a transaction block -- Supabase migrations run each file in a transaction, so use a separate" migration file for the index — aNOT VALID/VALIDATE CONSTRAINTsplit alone won't unblockCONCURRENTLY.🔧 Proposed split
ALTER TABLE public.inventory_count_drafts ADD COLUMN IF NOT EXISTS reviewed_at timestamptz, - ADD COLUMN IF NOT EXISTS reviewed_by uuid REFERENCES auth.users(id) ON DELETE SET NULL; - --- Covering index for the new FK, matching this repo's convention for every --- other *_by/*_user_id → auth.users(id) column (see --- idx_work_orders_completion_verified_by, idx_work_order_updates_updated_by_user_id). -CREATE INDEX IF NOT EXISTS idx_inventory_count_drafts_reviewed_by - ON public.inventory_count_drafts (reviewed_by); + ADD COLUMN IF NOT EXISTS reviewed_by uuid, + ADD CONSTRAINT inventory_count_drafts_reviewed_by_fkey + FOREIGN KEY (reviewed_by) REFERENCES auth.users(id) ON DELETE SET NULL NOT VALID; + +ALTER TABLE public.inventory_count_drafts + VALIDATE CONSTRAINT inventory_count_drafts_reviewed_by_fkey;Then, in a separate migration file (
CONCURRENTLYcannot share a transaction with any other statement):CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_inventory_count_drafts_reviewed_by ON public.inventory_count_drafts (reviewed_by);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql` around lines 19 - 27, Split the migration so the ALTER TABLE in the current migration adds reviewed_at and reviewed_by without creating the foreign key or index. Add the reviewed_by foreign key in a separate migration using the repository’s zero-downtime constraint approach, and create idx_inventory_count_drafts_reviewed_by with CREATE INDEX CONCURRENTLY in its own migration file containing no other statements.Source: Linters/SAST tools
unit/dexie/sync-outbox-backoff.test.ts (1)
87-172: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing coverage for the retryCount ≥ 3 skip-and-continue path.
All four scenarios here cover the "stop and reschedule" branch (
break) and the dead-letter branch, but none seed a mutation atretryCount: 2(so the next failure hits thenewRetryCount >= 3skip-and-continue branch) to verify a wake-up is still scheduled once the drain otherwise idles. This gap is what let the missingscheduleRetry()call in that branch (seelib/dexie/syncService.tslines 113-124) go undetected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@unit/dexie/sync-outbox-backoff.test.ts` around lines 87 - 172, Add a test in the “processOutbox — retry backoff” suite that seeds a mutation with retryCount: 2, forces its next push to fail, and verifies the retryCount ≥ 3 skip-and-continue path schedules exactly one wake-up timer when the drain becomes idle. Use the existing seedMutation, makeFakeSupabase, processOutbox, mutationRow, and fake-timer assertions, while preserving the dead-letter behavior covered by the existing test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(dashboard)/support-inbox/support-inbox-client.tsx:
- Line 382: Update the feedback date rendering in the support-inbox client to
use an explicit locale and timezone with toLocaleDateString, ensuring SSR and
hydration produce identical output while preserving the existing submitted_at
value.
In `@lib/dexie/context.tsx`:
- Around line 282-420: Reduce nesting in the v2 synchronization helpers by
extracting the innermost callbacks from scheduleV2Reconnect and runV2 into named
helpers declared alongside scheduleV2Reconnect. Specifically, move the
resubscribe failure handler, the checklists signal action, and the
TOKEN_REFRESHED setAuth failure handler out of their enclosing callbacks while
preserving their existing behavior and captured values, so nesting depth stays
within four levels.
In `@lib/dexie/syncService.ts`:
- Around line 113-124: Update processOutbox in lib/dexie/syncService.ts at lines
113-124 to call scheduleRetry(nextAttemptAt) before continuing from the
newRetryCount >= 3 branch. Add a test case in
unit/dexie/sync-outbox-backoff.test.ts at lines 87-172 that seeds retryCount: 2,
triggers the skip-and-continue path, and verifies a timer is scheduled and can
wake processing to reach the row.
In `@lib/inngest/functions/build-shopping-cart.ts`:
- Around line 149-156: The RateLimitError branch in the shopping-cart workflow
currently rethrows directly, discarding Kroger’s retry delay. Wrap the error in
Inngest’s RetryAfterError using RateLimitError.retryAfter converted from seconds
to milliseconds, and apply this consistently to Kroger-backed step.run paths
such as product search and cart-add.
In `@lib/inngest/functions/cron/notifications-retention.ts`:
- Line 74: Update the cron trigger object for the notifications retention
function to explicitly use the America/Chicago timezone and run at 9:30 AM local
time, preserving the stated schedule across CST and CDT. Ensure the trigger uses
the correct Inngest cron configuration key and schedule value.
In `@lib/kroger/client.ts`:
- Around line 65-67: Update the 429 handling around Retry-After in the client
response flow to support both non-negative numeric seconds and RFC HTTP-date
values. Parse numeric seconds first; when invalid, parse the date and calculate
the remaining delay in seconds, ensuring the resulting delay is valid and
non-negative. Use the 60-second default only when neither representation is
valid, then pass the resolved delay to RateLimitError.
In `@unit/inngest/cron-notifications-retention.test.ts`:
- Around line 24-26: Replace the untyped chain in the mocked Supabase setup
within the from mock with a small PromiseLike fluent-chain interface describing
the methods used by the test. Type the chain with that interface, remove the
explicit any eslint suppression, and preserve the existing mocked method
behavior.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 66-147: Add an explicit least-privilege permissions declaration at
the workflow level, setting GITHUB_TOKEN access to read-only repository contents
and disabling unnecessary scopes. Ensure the build, e2e, and db-invariants jobs
inherit these restricted permissions without granting write access to issues,
pull requests, contents, or other resources.
---
Nitpick comments:
In `@scripts/check-type-drift.mjs`:
- Around line 161-169: Configure a finite timeout for the RPC request in the
schema-report fetch, using the existing fetch options around the
db_type_shape_report call. Ensure a hung connection aborts promptly and causes
the CI step to fail rather than waiting for the runner timeout.
In
`@supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql`:
- Around line 19-27: Split the migration so the ALTER TABLE in the current
migration adds reviewed_at and reviewed_by without creating the foreign key or
index. Add the reviewed_by foreign key in a separate migration using the
repository’s zero-downtime constraint approach, and create
idx_inventory_count_drafts_reviewed_by with CREATE INDEX CONCURRENTLY in its own
migration file containing no other statements.
In `@unit/dexie/sync-outbox-backoff.test.ts`:
- Around line 87-172: Add a test in the “processOutbox — retry backoff” suite
that seeds a mutation with retryCount: 2, forces its next push to fail, and
verifies the retryCount ≥ 3 skip-and-continue path schedules exactly one wake-up
timer when the drain becomes idle. Use the existing seedMutation,
makeFakeSupabase, processOutbox, mutationRow, and fake-timer assertions, while
preserving the dead-letter behavior covered by the existing test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 72f145a8-dbc1-4a2a-afb8-85e847ad4d09
📒 Files selected for processing (39)
.env.example.github/workflows/ci.ymlCLAUDE.mdapp/(dashboard)/bookings/bookings-calendar.tsxapp/(dashboard)/bookings/bookings-client.tsxapp/(dashboard)/support-inbox/page.tsxapp/(dashboard)/support-inbox/support-inbox-client.tsxapp/api/inngest/route.tsapp/api/integrations/[provider]/callback/route.tsapp/connect/error/page.tsxapp/connect/finish/route.tsdocs/SCALABILITY_TIERS_REMAINING.mdlib/dexie/context.tsxlib/dexie/schema.tslib/dexie/sync/signals.tslib/dexie/syncService.tslib/inngest/functions/build-shopping-cart.tslib/inngest/functions/cron/notifications-retention.tslib/inngest/functions/hostaway/initial-sync.tslib/integrations/providers/hostaway.tslib/integrations/registry.tslib/kroger/client.tslib/rate-limit.tslib/sms/telnyx.tsscripts/check-type-drift.mjssupabase/migrations/20260725191358_crew_sync_broadcast_triggers.sqlsupabase/migrations/20260725200500_db_type_shape_report.sqlsupabase/migrations/20260725201000_add_vacancy_gap_suggestion_to_wo_source.sqlsupabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sqltypes/database.tsunit/dexie/fake-dexie.tsunit/dexie/sync-outbox-backoff.test.tsunit/dexie/sync-reconnect-jitter.test.tsunit/dexie/sync-signals.test.tsunit/inngest/build-shopping-cart.test.tsunit/inngest/cron-notifications-retention.test.tsunit/lib/kroger-client-rate-limit.test.tsunit/route-handlers/integrations-callback.test.tsunit/sms/send-sms-gate.test.ts
| </span> | ||
| <span style={{ fontSize: '11px', color: 'var(--text-muted)', flexShrink: 0 }}> | ||
| {new Date(f.created_at).toLocaleDateString()} | ||
| {new Date(f.submitted_at).toLocaleDateString()} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the feedback date deterministic across SSR and hydration.
toLocaleDateString() uses the server and browser locale/timezone defaults, so this can display different dates and trigger a hydration mismatch. Pass an explicit locale/timezone (or format after mount).
Proposed fix
- {new Date(f.submitted_at).toLocaleDateString()}
+ {new Date(f.submitted_at).toLocaleDateString('en-US', { timeZone: 'UTC' })}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {new Date(f.submitted_at).toLocaleDateString()} | |
| {new Date(f.submitted_at).toLocaleDateString('en-US', { timeZone: 'UTC' })} |
🧰 Tools
🪛 React Doctor (0.7.6)
[error] 382-382: This can cause a hydration mismatch because toLocaleDateString() formats with the server's locale and timezone during server rendering but the user's in the browser. Format it in a post-mount useEffect, or pass an explicit locale and timeZone.
Format locale/timezone-dependent values in a post-mount useEffect + state, or pass an explicit locale and timeZone so the server and the browser render the same text. Only runs on SSR-capable projects.
(no-locale-format-in-render)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(dashboard)/support-inbox/support-inbox-client.tsx at line 382, Update
the feedback date rendering in the support-inbox client to use an explicit
locale and timezone with toLocaleDateString, ensuring SSR and hydration produce
identical output while preserving the existing submitted_at value.
Source: Linters/SAST tools
Comment on lines
+113
to
124
| const nextAttemptAt = computeNextAttemptAt(newRetryCount, Date.now()) | ||
| await db.mutations.update(id, { retryCount: newRetryCount, nextAttemptAt }) | ||
| // Only block the queue on transient failures, not permanent ones. | ||
| // If we've retried >= 3 times, skip this mutation and continue | ||
| // draining so later mutations (which may be independent) still go through. | ||
| if (newRetryCount >= 3) continue | ||
| // Stop draining on first/second failure so later mutations against | ||
| // the same record aren't applied out of order. | ||
| // the same record aren't applied out of order; wake up again when | ||
| // the backoff window elapses. | ||
| this.scheduleRetry(nextAttemptAt) | ||
| break | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Skip-and-continue retries can get stranded, and no test catches it. processOutbox()'s "skip and continue" branch (retryCount >= 3) sets nextAttemptAt but never schedules a wake-up timer, unlike the sibling break branch — and the test suite never exercises that exact path to reveal the gap.
lib/dexie/syncService.ts#L113-L124: callthis.scheduleRetry(nextAttemptAt)beforecontinuein thenewRetryCount >= 3branch, matching thebreakbranch below it.unit/dexie/sync-outbox-backoff.test.ts#L87-L172: add a case seeding a mutation atretryCount: 2so the next failure hits the skip-and-continue branch, then assert a timer is still scheduled (vi.getTimerCount()and/oradvanceTimersByTimeAsyncreaching the row) once the drain otherwise completes.
📍 Affects 2 files
lib/dexie/syncService.ts#L113-L124(this comment)unit/dexie/sync-outbox-backoff.test.ts#L87-L172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/dexie/syncService.ts` around lines 113 - 124, Update processOutbox in
lib/dexie/syncService.ts at lines 113-124 to call scheduleRetry(nextAttemptAt)
before continuing from the newRetryCount >= 3 branch. Add a test case in
unit/dexie/sync-outbox-backoff.test.ts at lines 87-172 that seeds retryCount: 2,
triggers the skip-and-continue path, and verifies a timer is scheduled and can
wake processing to reach the row.
Comment on lines
+149
to
+156
| if (err instanceof RateLimitError) { | ||
| // Kroger's own API quota (or our proactive guard in front of it, | ||
| // see lib/kroger/client.ts's krogerFetch) is exhausted. Rethrow | ||
| // so Inngest retries this step with backoff instead of silently | ||
| // degrading to the list-only fallback below — that would mask a | ||
| // transient condition as a permanent one and skip a retry that | ||
| // would likely succeed once the window resets. | ||
| throw err |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'build-shopping-cart\.ts|kroger/client\.ts|lib/inngest/functions' . | sed 's#^\./##'
echo "== build-shopping-cart outline =="
ast-grep outline lib/inngest/functions/build-shopping-cart.ts --view expanded || true
echo "== relevant build-shopping-cart sections =="
nl -ba lib/inngest/functions/build-shopping-cart.ts | sed -n '1,260p'
echo "== kroger fetch references =="
rg -n "krogerFetch|RateLimitError|RetryAfterError|retryAfter|buildShoppingCart|function buildShoppingCart|retries:" lib package.json || true
echo "== package inngest versions =="
if [ -f package.json ]; then jq '.dependencies.inngest, .devDependencies.inngest' package.json; fiRepository: smj1860/fieldstay
Length of output: 521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | grep -E '(^lib/inngest/functions/build-shopping-cart\.ts$|kroger/client\.ts$|package\.json$|tsconfig\.json$)' || true
echo "== build-shopping-cart relevant contents =="
awk '{printf "%6d\t%s\n", NR, $0}' lib/inngest/functions/build-shopping-cart.ts | sed -n '1,240p'
echo "== kroger/client relevant contents =="
awk '{printf "%6d\t%s\n", NR, $0}' lib/kroger/client.ts | sed -n '1,220p'
echo "== inngest references =="
rg -n "RateLimitError|RetryAfterError|retryAfter|krogerFetch|buildShoppingCart|retries:" lib lib/inngest/functions lib/kroger package.json || true
echo "== package inngest versions =="
if [ -f package.json ]; then
python3 - <<'PY' | sed -n '1,40p'
import json
p=json.load(open('package.json'))
print('dependencies:', p.get('dependencies',{}).get('inngest'))
print('devDependencies:', p.get('devDependencies',{}).get('inngest'))
PY
fiRepository: smj1860/fieldstay
Length of output: 47254
Preserve Kroger’s Retry-After for Inngest retries.
RateLimitError only has a seconds-based retryAfter; plain rethrows let Inngest retry with its default backoff and can exhaust function retries before Kroger’s quota resets. Have Kroger’s error path throw Inngest’s RetryAfterError with retryAfter * 1000 ms, or handle Kroger rate limits by sleeping step.sleep(..., ...s) before retrying in each Kroger-backed step.run, including product search and cart-add.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/inngest/functions/build-shopping-cart.ts` around lines 149 - 156, The
RateLimitError branch in the shopping-cart workflow currently rethrows directly,
discarding Kroger’s retry delay. Wrap the error in Inngest’s RetryAfterError
using RateLimitError.retryAfter converted from seconds to milliseconds, and
apply this consistently to Kroger-backed step.run paths such as product search
and cart-add.
| name: 'Cron: Notifications Retention Purge', | ||
| retries: 1, | ||
| }, | ||
| { cron: '30 14 * * *' }, // 15 min after dailyGuestPiiRetention |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'notifications-retention\.ts$' . || true
echo "== file excerpt if present =="
file="$(fd 'notifications-retention\.ts$' . | head -n1 || true)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,130p' "$file" | cat -n
fi
echo "== inngest cron config refs =="
rg -n "notifications-retention|dailyGuestPiiRetention|America/Chicago|timezone|TZ=|\\(cron:" --glob '!node_modules' --glob '!dist' --glob '!build' . || true
echo "== package versions =="
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
[ -f "$f" ] && echo "--- $f ---" && (sed -n '1,160p' "$f"; true)
doneRepository: smj1860/fieldstay
Length of output: 34056
🌐 Web query:
Inngest v3 TypeScript cron timezone-prefixed expressions TZ America/Chicago
💡 Result:
In Inngest, you can specify a timezone for cron triggers by adding a TZ prefix to your cron expression [1][2][3]. To use the America/Chicago timezone, you should use the prefix TZ=America/Chicago [3][4]. You can implement this in your TypeScript function using the cron helper or by passing a trigger object [2][5]. Example using the cron helper: import { inngest, cron } from "inngest"; inngest.createFunction( { id: "my-scheduled-job", triggers: [cron("TZ=America/Chicago 0 9 * * 1-5")], }, async ({ step }) => { // Runs at 9:00 AM CT on weekdays }); Example using a trigger object: inngest.createFunction( { id: "my-scheduled-job", triggers: [{ cron: "TZ=America/Chicago 0 9 * * 1-5" }], }, async ({ step }) => { // Runs at 9:00 AM CT on weekdays }); Important considerations: - Daylight Saving Time (DST): Inngest's cron behavior follows the underlying cron library and does not apply special DST correction [3][6]. Schedules near transition times (such as 2:00 AM in many US regions) can behave unexpectedly, potentially running zero, one, or two times depending on the transition [3][6]. To ensure consistent execution timing, Inngest recommends avoiding transition-hour schedules or preferring UTC when possible [3][6].
Citations:
- 1: https://www.inngest.com/docs/reference/typescript/v3/functions/create
- 2: https://www.inngest.com/docs/reference/typescript/functions/triggers
- 3: https://www.inngest.com/docs/guides/scheduled-functions
- 4: https://www.inngest.com/uses/scheduled-jobs
- 5: https://www.inngest.com/docs/reference/typescript/v4/functions/triggers
- 6: https://www.inngest.com/patterns/running-code-on-a-schedule
Preserve the stated 9:30 AM Central schedule.
30 14 * * * runs at 9:30 AM during CDT but 8:30 AM during CST. Use chron: 'TZ=America/Chicago 30 9 * * *' with Inngest’s cron trigger object, or correct the stated schedule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/inngest/functions/cron/notifications-retention.ts` at line 74, Update the
cron trigger object for the notifications retention function to explicitly use
the America/Chicago timezone and run at 9:30 AM local time, preserving the
stated schedule across CST and CDT. Ensure the trigger uses the correct Inngest
cron configuration key and schedule value.
Comment on lines
+65
to
+67
| if (res.status === 429) { | ||
| const retryAfter = parseInt(res.headers.get('Retry-After') ?? '60', 10) | ||
| throw new RateLimitError(retryAfter) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)lib/kroger/client\.ts$|(^|/)kroger' || true
echo
echo "== file excerpt =="
if [ -f lib/kroger/client.ts ]; then
nl -ba lib/kroger/client.ts | sed -n '1,130p'
fi
echo
echo "== usages of RateLimitError / krogerFetch =="
rg -n "RateLimitError|krogerFetch|Retry-After" lib pkg ts app worker || true
echo
echo "== behavioral probe: current parseInt behavior for Retry-After forms =="
node - <<'JS'
function currentRetryAfter(resHeader) {
const retryAfter = parseInt(resHeader ?? '60', 10)
return retryAfter
}
for (const value of ['60', '0', '61', 'Thu, 01 Jan 2026 00:00:01 GMT', '20260701T000000Z', '']) {
console.log(JSON.stringify(value), "=>", currentRetryAfter(value))
}
JSRepository: smj1860/fieldstay
Length of output: 477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
if [ -f lib/kroger/client.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' lib/kroger/client.ts | sed -n '1,140p'
fi
echo
echo "== usages of RateLimitError / krogerFetch / Retry-After =="
rg -n "RateLimitError|krogerFetch|Retry-After" lib pkg ts app worker || true
echo
echo "== behavioral probe: current parseInt behavior for Retry-After forms =="
node - <<'JS'
function currentRetryAfter(resHeader) {
const retryAfter = parseInt(resHeader ?? '60', 10)
return retryAfter
}
for (const value of ['60', '0', '61', 'Thu, 01 Jan 2026 00:00:01 GMT', '20260701T000000Z', '']) {
console.log(JSON.stringify(value), "=>", currentRetryAfter(value))
}
JSRepository: smj1860/fieldstay
Length of output: 12466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== RateLimitError definition =="
awk '{printf "%6d\t%s\n", NR, $0}' lib/integrations/types.ts | sed -n '260,335p'
echo
echo "== Kroger caller exception handling excerpt =="
awk '{printf "%6d\t%s\n", NR, $0}' lib/inngest/functions/build-shopping-cart.ts | sed -n '138,165p'
echo
echo "== Kroger-specific rate limit tests if present =="
if [ -f unit/lib/kroger-client-rate-limit.test.ts ]; then
awk '{printf "%6d\t%s\n", NR, $0}' unit/lib/kroger-client-rate-limit.test.ts | sed -n '1,220p'
else
echo "unit/lib/kroger-client-rate-limit.test.ts not found"
fi
echo
echo "== exact krogerFetch Retry-After usages =="
awk '{printf "%6d\t%s\n", NR, $0}' lib/kroger/client.ts | sed -n '60,75p'Repository: smj1860/fieldstay
Length of output: 12140
Handle HTTP-date Retry-After values.
parseInt() returns NaN for RFC-compliant date-form Retry-After values, so the Kroger RateLimitError can carry an invalid retry delay. Parse non-negative seconds first, then fall back to Date.parse()/remaining seconds; default to 60 only when neither is valid.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 66-66: Prefer Number.parseInt over parseInt.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/kroger/client.ts` around lines 65 - 67, Update the 429 handling around
Retry-After in the client response flow to support both non-negative numeric
seconds and RFC HTTP-date values. Parse numeric seconds first; when invalid,
parse the date and calculate the remaining delay in seconds, ensuring the
resulting delay is valid and non-negative. Use the 60-second default only when
neither representation is valid, then pass the resolved delay to RateLimitError.
Source: Linters/SAST tools
Comment on lines
+24
to
+26
| const from = vi.fn((table: string) => { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const chain: any = {} |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the untyped fluent mock.
Define a small PromiseLike chain interface for the mocked Supabase methods instead of const chain: any.
As per coding guidelines, **/*.{ts,tsx} says “Do not use any, as any, or @ts-ignore.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@unit/inngest/cron-notifications-retention.test.ts` around lines 24 - 26,
Replace the untyped chain in the mocked Supabase setup within the from mock with
a small PromiseLike fluent-chain interface describing the methods used by the
test. Type the chain with that interface, remove the explicit any eslint
suppression, and preserve the existing mocked method behavior.
Source: Coding guidelines
SonarCloud (13 findings on this branch): - lib/dexie/context.tsx: extract 3 deeply-nested closures (scheduleV2Reconnect's retry, runV2's checklist sync, the v2 auth-state-change handler) into named sibling functions to bring nesting depth back under the project's limit - lib/dexie/sync/signals.ts, lib/dexie/syncService.ts: NOSONAR the two Math.random() jitter sites (already eslint-disable-justified as timing jitter, not id/token generation) - lib/kroger/client.ts: Number.parseInt over parseInt - lib/sms/telnyx.ts: extract formatOfferPrice/formatPercentageOffer/ formatFixedAmountOffer to remove the nested ternaries and bring formatOffer's cognitive complexity back under 15 - scripts/check-type-drift.mjs: NOSONAR the three regex findings — this script only ever parses our own committed types/database.ts, never attacker-controlled input, so ReDoS isn't a real risk here - supabase/migrations/20260725200500_db_type_shape_report.sql: dedupe the 3x-repeated 'public' literal via a target_schema CTE; verified byte-identical md5 output on both projects before/after re-applying CI gate gaps surfaced by PR #508's own db-invariants job, both from PR #507 (guidebook v2) never having its migration pushed to the E2E project: - Applied 20260726100000_guidebook_v2_foundation.sql and 20260726120000_guidebook_property_photos_storage_policies.sql to E2E (already live on production) — clears the type-drift findings for guidebook_offer_redemptions and hero_photo_storage_path - New migration: covering index for guidebook_offer_redemptions.booking_id, the one FK column that migration's own indexes didn't cover — clears the FK-coverage invariant finding that surfaced once the table existed on E2E Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
Owner
Author
Flagging: 4 e2e failures are pre-existing, not caused by this PRThe
All four are the same shape: a server mutation completes, but its effect doesn't show up in the UI within the test's timeout. 61 other specs pass every run. This is not a regression from this PR. Confirmed by:
This is a chronic, pre-existing environmental flake in the shared E2E Supabase project ( Merging with this pre-existing failure acknowledged. Generated by Claude Code |
…lability-lgfh15 # Conflicts: # lib/sms/telnyx.ts
|
smj1860
pushed a commit
that referenced
this pull request
Jul 26, 2026
Both docs still showed Phases 2-4 as not-done even though they merged via PR #508 on 2026-07-26 — verified all three are live on main (broadcast trigger migration applied to both Supabase projects, client cutover flag in lib/dexie/context.tsx, outbox backoff in syncService.ts). Only Phase 5 (rollout: two-device acceptance test, production flag flip, soak, old-code deletion) remains open. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp
This was referenced Jul 29, 2026
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit