Skip to content

feat: add race condition test for calendar-cache-serve booking flow#22170

Merged
hbjORbj merged 14 commits intomainfrom
devin/1751323069-reproduce-booking-race-condition
Jul 9, 2025
Merged

feat: add race condition test for calendar-cache-serve booking flow#22170
hbjORbj merged 14 commits intomainfrom
devin/1751323069-reproduce-booking-race-condition

Conversation

@zomars
Copy link
Copy Markdown
Contributor

@zomars zomars commented Jun 30, 2025

Add Playwright test to reproduce booking race condition with calendar cache

refs #22065

Summary

This PR adds a Playwright end-to-end test that ensures we prevent a race condition in Cal.com's booking flow for team events with round-robin host assignment. The test demonstrates how two guests could book the same timeslot within 1-2 seconds when the calendar-cache-serve feature flag is enabled, causing a double-booking issue due to stale calendar cache data.

Key Implementation:

  • Creates team event with SchedulingType.ROUND_ROBIN scheduling
  • Enables both calendar-cache and calendar-cache-serve feature flags via prisma.teamFeatures.createMany()
  • Uses parallel browser contexts to simulate Guest A and Guest B booking simultaneously
  • Applies 1.5 second delay between booking attempts to reproduce the race condition window
  • Verifies both bookings succeed and queries database to confirm double-booking occurred

Test Results:
The test successfully reproduces the bug locally, consistently showing output like:

Race condition successfully reproduced - both bookings created for same timeslot: {
  sameTimeslot: true,
  host1: 260,
  host2: 261,
  timeslot: '2025-08-01T08:00:00.000Z'
}

Review & Testing Checklist for Human

  • Verify race condition reproduction: Run the test locally using TZ=UTC PLAYWRIGHT_HEADLESS=1 yarn e2e apps/web/playwright/booking-race-condition.e2e.ts and confirm it consistently reproduces the double-booking scenario with different hosts assigned to the same timeslot
  • Test isolation and cleanup: Confirm the test doesn't interfere with other E2E tests by running a broader test suite and checking that database records are properly isolated/cleaned up
  • Review CI failure context: Verify that the E2E (1/4) CI failure is indeed unrelated to this race condition test (investigation shows navigation timeout errors in ab-tests-redirect.e2e.ts and app-store.e2e.ts while this test is not in that shard)
  • Validate feature flag logic: Ensure the test correctly enables both required feature flags (calendar-cache and calendar-cache-serve) and that this matches the production scenario where the race condition occurs

Diagram

%%{ init : { "theme" : "default" }}%%
graph TB
    Test["apps/web/playwright/booking-race-condition.e2e.ts"]:::major-edit
    Fixtures["apps/web/playwright/lib/fixtures.ts"]:::context
    TestUtils["apps/web/playwright/lib/testUtils.ts"]:::context
    
    Test --> Fixtures
    Test --> TestUtils
    Test --> PrismaFeatures["prisma.teamFeatures.createMany()"]:::context
    Test --> BookingAPI["/api/book/event"]:::context
    Test --> CalendarCache["packages/features/calendar-cache/"]:::context
    
    Test --> Context1["Browser Context 1 (Guest A)"]:::context
    Test --> Context2["Browser Context 2 (Guest B)"]:::context
    
    Context1 --> BookingFlow1["Booking Flow"]:::context
    Context2 --> BookingFlow2["Booking Flow"]:::context
    
    BookingFlow1 --> Database["Database Verification"]:::context
    BookingFlow2 --> Database
    
    subgraph Legend
        L1["Major Edit"]:::major-edit
        L2["Minor Edit"]:::minor-edit  
        L3["Context/No Edit"]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • Purpose: This test is designed to prove the bug exists, not fix it. A follow-up PR will tighten the cache TTL to resolve the race condition.
  • CI Status: The E2E (1/4) failure in CI appears unrelated to this test based on investigation. E2E shards (2/4), (3/4), and (4/4) all pass, and the failing shard contains navigation timeout errors in different test files.
  • Local Testing: The test passes locally with return code 0 and successfully demonstrates the race condition by showing different hosts (e.g., 260 vs 261) assigned to the same timeslot.

Link to Devin run: https://app.devin.ai/sessions/02516b4cc3374102a0d73a11d665ab62
Requested by: zomars (@zomars)

- Reproduces double-booking when two guests book same timeslot 1-2s apart
- Enables both calendar-cache and calendar-cache-serve feature flags
- Uses parallel browser contexts to simulate concurrent booking attempts
- Verifies both bookings succeed and create double-booking scenario

Co-Authored-By: zomars@cal.com <zomars@me.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@keithwillcode keithwillcode added core area: core, team members only dba foundation labels Jun 30, 2025
@vercel
Copy link
Copy Markdown

vercel bot commented Jun 30, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2025 6:20pm
cal-eu ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2025 6:20pm

@delve-auditor
Copy link
Copy Markdown

delve-auditor bot commented Jun 30, 2025

No security or compliance issues detected. Reviewed everything up to e92a959.

Security Overview
  • 🔎 Scanned files: 1 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► booking-race-condition.e2e.ts
    Add concurrent booking test with round-robin scheduling

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 30, 2025

E2E results are ready!

- Change page.waitForTimeout(1500) to use correct page context
- Fix timeslot selection to ensure both contexts book same slot
- Remove incorrect host assertion and add logging to demonstrate race condition
- Test now successfully reproduces double-booking bug locally
- Fixes 'end of central directory record signature not found' error

Co-Authored-By: zomars@cal.com <zomars@me.com>
@vercel vercel bot temporarily deployed to Preview – api July 1, 2025 19:53 Inactive
@vercel vercel bot temporarily deployed to Preview – api July 8, 2025 00:34 Inactive
@zomars zomars marked this pull request as ready for review July 8, 2025 02:15
@graphite-app graphite-app bot requested a review from a team July 8, 2025 02:15
@dosubot dosubot bot added automated-tests area: unit tests, e2e tests, playwright bookings area: bookings, availability, timezones, double booking labels Jul 8, 2025
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cubic found 2 issues across 1 file. Review them in cubic.dev

React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.


const teamMemberships = await prisma.membership.findMany({
where: { teamId: team.id },
include: { user: true },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using include with Prisma returns all columns of the related user table, which violates the project guideline to avoid include and only fetch required fields via select, leading to unnecessary data exposure and performance overhead.

@graphite-app
Copy link
Copy Markdown

graphite-app bot commented Jul 8, 2025

Graphite Automations

"Add foundation team as reviewer" took an action on this PR • (07/08/25)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link
Copy Markdown
Contributor

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩

@hbjORbj hbjORbj merged commit 179a547 into main Jul 9, 2025
41 checks passed
@hbjORbj hbjORbj deleted the devin/1751323069-reproduce-booking-race-condition branch July 9, 2025 14:33
Arephan pushed a commit to Arephan/cal.com that referenced this pull request Jul 11, 2025
…alcom#22170)

* feat: add race condition test for calendar-cache-serve booking flow

- Reproduces double-booking when two guests book same timeslot 1-2s apart
- Enables both calendar-cache and calendar-cache-serve feature flags
- Uses parallel browser contexts to simulate concurrent booking attempts
- Verifies both bookings succeed and create double-booking scenario

Co-Authored-By: zomars@cal.com <zomars@me.com>

* fix: correct page reference in race condition test timeout

- Change page.waitForTimeout(1500) to use correct page context
- Fix timeslot selection to ensure both contexts book same slot
- Remove incorrect host assertion and add logging to demonstrate race condition
- Test now successfully reproduces double-booking bug locally
- Fixes 'end of central directory record signature not found' error

Co-Authored-By: zomars@cal.com <zomars@me.com>

* Update booking-race-condition.e2e.ts

* Update booking-race-condition.e2e.ts

* Update booking-race-condition.e2e.ts

* Update booking-race-condition.e2e.ts

* Create scratchpad.md

* Update booking-race-condition.e2e.ts

* Delete scratchpad.md

* Update booking-race-condition.e2e.ts

* Update booking-race-condition.e2e.ts

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-tests area: unit tests, e2e tests, playwright bookings area: bookings, availability, timezones, double booking core area: core, team members only dba foundation ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants