Skip to content

fix(booking): lifecycle correctness — cancelled-event CAS, class partial reschedule, cleanup guards, utilization re-key, reminder scheduler - #1002

Merged
teetangh merged 3 commits into
devfrom
fix/booking-lifecycle-correctness
Jul 17, 2026
Merged

fix(booking): lifecycle correctness — cancelled-event CAS, class partial reschedule, cleanup guards, utilization re-key, reminder scheduler#1002
teetangh merged 3 commits into
devfrom
fix/booking-lifecycle-correctness

Conversation

@teetangh

@teetangh teetangh commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Problem

The 2026-07-17 booking-lifecycle audit (run alongside PR #998) verified five defects in the reschedule/cancel/cleanup path with file-level evidence. This PR fixes the ones that are independent of the refund-gateway work.

Fixes (audit ids)

  • C2 — cancelled-event resurrection. SlotAllocationService.updateEventStatus used unguarded tx.webinar.update / tx.class.update to stamp SCHEDULED, so an allocation racing a cancel could resurrect a CANCELLED (or re-open a COMPLETED) webinar or class. The EVENT_ALLOWED_FROM map existed in lib/booking/transitions.ts but was never applied; new transitionWebinarEvent / transitionClassEvent helpers bake it into the UPDATE's WHERE clause, and a zero-row match throws IllegalTransitionError, rolling back the allocation — exactly the [B2C][SUBSCRIPTIONS] Approval transition read-then-decide race in subscription route #836 doctrine the request-status types already follow.
  • R1 — class partial reschedule silently escalated. The slotIds[] branch of the reschedule route was SUBSCRIPTION-only, so a per-session class reschedule fell through to the whole-class branch: the 24-hour policy validated the wrong slot set and every session in the class was marked tentative. The slotIds filter and tentative-marking branches now cover CLASS identically to SUBSCRIPTION.
  • R3 — rescheduled slots got zero cleanup grace. cleanup-tentative-slots measured its grace window from slot.createdAt, but a reschedule flips isTentative on an old row — so rescheduled slots were already "stale" at the next sweep. Grace now runs from updatedAt (the tentative-marking write). The active-review guard also only covered consultation/subscription statuses; SCHEDULED/IN_PROGRESS webinars and classes mid-reschedule are now skipped instead of having their slots (and attendee links) deleted.
  • M4 — org utilization double-debit on re-allocation. Re-allocation deletes counted appointments and recreates them under fresh ids, so the BookingUtilization.appointmentIds dedup saw every replaced session as new and debited the program cap again. Stale tracked ids are now substituted one-for-one with the incoming ids without debiting; only genuinely additional sessions debit.
  • Reminder scheduler was missing entirely. send-appointment-reminders existed at all three layers (script, job wrapper, HTTP route) but nothing scheduled it — reminders never sent. Added the hourly GitHub Actions workflow mirroring the repo's cron pattern (the 45–75-minute reminder window assumes at-least-hourly firing; Redis SET-NX in the script dedupes overlaps).

Testing

657 tests green across __tests__/booking-algorithm, __tests__/schedule, and the enterprise cap suites, including updated assertions that pin the new WHERE-guarded transitions. Full tsc --noEmit clean. __tests__/payments/razorpay-refund-target.test.ts fails identically on clean dev in this environment (pre-existing, unrelated).

Related

Follow-up issues for the audit's remaining findings (refund-dependent flows, UI kind-gates, proration, dispute freeze, QStash, terminology) are filed separately and linked from the audit umbrella. The refund-gateway fix (M1/M8) ships in its own PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Appointment reminders now run automatically every hour, with an option for manual triggering and operational failure alerts.
    • Rescheduling class bookings with selected sessions now affects only those sessions.
  • Bug Fixes

    • Tentative slots now remain protected based on their last update, reducing premature cleanup.
    • Improved event status transition safety for webinars and classes.
    • Prevented duplicate subscription-cap deductions during slot reallocations.

…ial reschedule, cleanup guards, utilization re-key, reminder scheduler

C2: webinar/class allocation now rides WHERE-guarded transitions
(EVENT_ALLOWED_FROM) so a cancel racing an allocation can no longer
resurrect a CANCELLED event. R1: the slotIds reschedule branch covers
CLASS, ending the silent escalation of a per-session class reschedule
to the whole class. R3: tentative-slot cleanup measures grace from the
last write (rescheduled slots had zero grace) and skips SCHEDULED/
IN_PROGRESS webinars and classes mid-reschedule. M4: BookingUtilization
substitutes re-created appointment ids one-for-one instead of
re-debiting every re-allocation. The appointment-reminder job finally
gets a scheduler (hourly GH Actions workflow; all three layers existed
with nothing firing them).

Findings C2/R1/R3/M4 + reminder gap from the 2026-07-17 booking audit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds hourly appointment reminder automation and updates booking transitions, subscription allocation accounting, class rescheduling, and tentative-slot cleanup to use guarded state and timestamp-aware behavior.

Changes

Appointment reminder automation

Layer / File(s) Summary
Reminder workflow execution
.github/workflows/send-appointment-reminders.yml
Adds scheduled and manual workflow triggers, service environment variables, Prisma setup, reminder execution, and Slack failure notification handling.

Booking lifecycle hardening

Layer / File(s) Summary
Guarded webinar and class transitions
lib/booking/transitions.ts, utils/slotAllocation/SlotAllocationService.ts, __tests__/booking-algorithm/slotAllocationService.test.ts
Adds guarded updateMany transitions for webinar and class events and updates status-transition tests accordingly.
Subscription cap reallocation
utils/slotAllocation/SlotAllocationService.ts
Uses tracked appointment IDs to avoid repeated cap debits during subscription reallocation.
Partial rescheduling and tentative-slot cleanup
app/api/appointments/[appointmentId]/reschedule/route.ts, scripts/appointments/cleanup-tentative-slots.ts, __tests__/booking*/**
Extends slot-specific rescheduling to classes, expires tentative slots from updatedAt, filters active webinar/class events, and updates fixtures.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant ReminderScript
  participant Database
  participant Novu
  participant SlackOps
  GitHubActions->>ReminderScript: Run appointment reminder job
  ReminderScript->>Database: Read appointment reminders
  ReminderScript->>Novu: Send notifications
  GitHubActions->>SlackOps: Notify on workflow failure
Loading

Possibly related issues

Possibly related PRs

Suggested reviewers: shubham79

Poem

I hop through slots where timestamps glow,
Guarded states now tell me where to go.
Classes reschedule, stale holds depart,
Reminders ping with a rabbit’s heart.
CI watches softly through the night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main booking lifecycle fixes and the new reminder scheduler.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/booking-lifecycle-correctness

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploy Preview for familiarise ready!

Name Link
🔨 Latest commit c3d2dbf
🔍 Latest deploy log https://app.netlify.com/projects/familiarise/deploys/6a5a300ce85c890008a07b4c
😎 Deploy Preview https://deploy-preview-1002--familiarise.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 83 (🟢 up 28 from production)
Accessibility: 99 (🟢 up 3 from production)
Best Practices: 92 (🟢 up 9 from production)
SEO: 100 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces guarded transitions for webinar and class events to prevent race conditions (such as resurrecting cancelled or completed events during concurrent allocation) by replacing direct updates with status-guarded updateMany calls. It also refactors the reschedule API to support per-session class reschedules, updates the tentative slot cleanup script to measure grace periods from updatedAt instead of createdAt (and avoids sweeping active group events), and optimizes the booking utilization logic to prevent double-debiting during re-allocation. No review comments were provided, so there is no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

The sweep now measures grace from updatedAt; the #829 guard fixture
predated the field and threw before the delete ran.

Part of #1002.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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 @.github/workflows/send-appointment-reminders.yml:
- Around line 27-34: Update the workflow’s “Checkout code” and “Setup Node.js”
steps to reference immutable, full-length commit SHAs instead of the mutable `@v5`
tags, and add persist-credentials: false to the checkout step’s with
configuration.
- Around line 27-34: Update the checkout and Node.js setup steps in the workflow
to reference immutable, exact commit SHAs instead of the mutable `@v5` tags. Add
persist-credentials: false to the actions/checkout configuration, while
preserving the existing Node.js version and npm cache settings.
- Around line 8-12: Update the workflow around the top-level workflow_dispatch
and send-appointment-reminders job: declare workflow permissions with contents
read only, add a concurrency group to prevent overlapping runs, and set a
descriptive name on the send-appointment-reminders job for GitHub Actions
display.
- Around line 8-12: Update the workflow definition around the
send-appointment-reminders job by explicitly setting workflow-level permissions
to contents: read, adding a concurrency group that prevents overlapping runs,
and assigning a descriptive name to the send-appointment-reminders job for
GitHub Actions display.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 144fc2c1-3cd4-402b-b068-0f8416b75027

📥 Commits

Reviewing files that changed from the base of the PR and between 7a447c6 and c3d2dbf.

📒 Files selected for processing (8)
  • .github/workflows/send-appointment-reminders.yml
  • __tests__/booking-algorithm/rescheduleCancel.test.ts
  • __tests__/booking-algorithm/slotAllocationService.test.ts
  • __tests__/booking/cleanup-tentative-guard.test.ts
  • app/api/appointments/[appointmentId]/reschedule/route.ts
  • lib/booking/transitions.ts
  • scripts/appointments/cleanup-tentative-slots.ts
  • utils/slotAllocation/SlotAllocationService.ts

Comment on lines +8 to +12
workflow_dispatch: # Allow manual triggering

jobs:
send-appointment-reminders:
runs-on: ubuntu-latest

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Restrict workflow permissions and prevent concurrent executions.

Address static analysis warnings and follow best practices by making these workflow-level improvements:

  1. Permissions: Explicitly declare permissions: contents: read to adhere to the principle of least privilege.
  2. Concurrency: Add a concurrency group to prevent overlapping workflow runs (which complements your Redis SET-NX lock).
  3. Job Name: Add a name to the job for better display in the GitHub Actions UI.

As per static analysis hints, the workflow defaults to overly broad permissions, lacks job-level concurrency limits, and contains a job definition without a name.

🔒 Proposed fixes for workflow definition
   workflow_dispatch: # Allow manual triggering
 
+permissions:
+  contents: read
+
+concurrency:
+  group: ${{ github.workflow }}
+
 jobs:
   send-appointment-reminders:
+    name: Send Appointment Reminders
     runs-on: ubuntu-latest
📝 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
workflow_dispatch: # Allow manual triggering
jobs:
send-appointment-reminders:
runs-on: ubuntu-latest
workflow_dispatch: # Allow manual triggering
permissions:
contents: read
concurrency:
group: ${{ github.workflow }}
jobs:
send-appointment-reminders:
name: Send Appointment Reminders
runs-on: ubuntu-latest
🧰 Tools
🪛 zizmor (1.26.1)

[info] 11-11: workflow or action definition without a name (anonymous-definition): this job

(anonymous-definition)

🤖 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/send-appointment-reminders.yml around lines 8 - 12, Update
the workflow around the top-level workflow_dispatch and
send-appointment-reminders job: declare workflow permissions with contents read
only, add a concurrency group to prevent overlapping runs, and set a descriptive
name on the send-appointment-reminders job for GitHub Actions display.

Source: Linters/SAST tools


🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Restrict workflow permissions and prevent concurrent executions.

Address static analysis warnings and follow best practices by making these workflow-level improvements:

  1. Permissions: Explicitly declare permissions: contents: read to adhere to the principle of least privilege.
  2. Concurrency: Add a concurrency group to prevent overlapping workflow runs (which complements your Redis SET-NX lock).
  3. Job Name: Add a name to the job for better display in the GitHub Actions UI.

As per static analysis hints, the workflow defaults to overly broad permissions, lacks job-level concurrency limits, and contains a job definition without a name.

🔒 Proposed fixes for workflow definition
   workflow_dispatch: # Allow manual triggering
 
+permissions:
+  contents: read
+
+concurrency:
+  group: ${{ github.workflow }}
+
 jobs:
   send-appointment-reminders:
+    name: Send Appointment Reminders
     runs-on: ubuntu-latest
📝 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
workflow_dispatch: # Allow manual triggering
jobs:
send-appointment-reminders:
runs-on: ubuntu-latest
workflow_dispatch: # Allow manual triggering
permissions:
contents: read
concurrency:
group: ${{ github.workflow }}
jobs:
send-appointment-reminders:
name: Send Appointment Reminders
runs-on: ubuntu-latest
🧰 Tools
🪛 zizmor (1.26.1)

[info] 11-11: workflow or action definition without a name (anonymous-definition): this job

(anonymous-definition)

🤖 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/send-appointment-reminders.yml around lines 8 - 12, Update
the workflow definition around the send-appointment-reminders job by explicitly
setting workflow-level permissions to contents: read, adding a concurrency group
that prevents overlapping runs, and assigning a descriptive name to the
send-appointment-reminders job for GitHub Actions display.

Source: Linters/SAST tools

Comment on lines +27 to +34
- name: Checkout code
uses: actions/checkout@v5

- name: Setup Node.js
uses: actions/setup-node@v5
with:
node-version: "22"
cache: "npm"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Secure Action references and checkout credentials.

To mitigate supply chain risks and address static analysis errors/warnings:

  1. Pin actions: Pin actions/checkout and actions/setup-node to exact commit SHAs instead of mutable version tags (like @v5).
  2. Persist credentials: Set persist-credentials: false on the checkout action to prevent the workflow from unnecessarily saving the GitHub token in the local Git configuration.

As per static analysis hints, actions are not pinned to a hash, and there is credential persistence through GitHub Actions artifacts because persist-credentials: false is not set.

🔒 Proposed fixes for action references
       - name: Checkout code
-        uses: actions/checkout@v5
+        uses: actions/checkout@<commit-sha> # e.g., actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
+        with:
+          persist-credentials: false
 
       - name: Setup Node.js
-        uses: actions/setup-node@v5
+        uses: actions/setup-node@<commit-sha>
         with:
           node-version: "22"
           cache: "npm"
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 27-28: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 31-31: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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/send-appointment-reminders.yml around lines 27 - 34,
Update the workflow’s “Checkout code” and “Setup Node.js” steps to reference
immutable, full-length commit SHAs instead of the mutable `@v5` tags, and add
persist-credentials: false to the checkout step’s with configuration.

Source: Linters/SAST tools


🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Secure Action references and checkout credentials.

To mitigate supply chain risks and address static analysis errors/warnings:

  1. Pin actions: Pin actions/checkout and actions/setup-node to exact commit SHAs instead of mutable version tags (like @v5).
  2. Persist credentials: Set persist-credentials: false on the checkout action to prevent the workflow from unnecessarily saving the GitHub token in the local Git configuration.

As per static analysis hints, actions are not pinned to a hash, and there is credential persistence through GitHub Actions artifacts because persist-credentials: false is not set.

🔒 Proposed fixes for action references
       - name: Checkout code
-        uses: actions/checkout@v5
+        uses: actions/checkout@<commit-sha> # e.g., actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
+        with:
+          persist-credentials: false
 
       - name: Setup Node.js
-        uses: actions/setup-node@v5
+        uses: actions/setup-node@<commit-sha>
         with:
           node-version: "22"
           cache: "npm"
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 27-28: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 31-31: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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/send-appointment-reminders.yml around lines 27 - 34,
Update the checkout and Node.js setup steps in the workflow to reference
immutable, exact commit SHAs instead of the mutable `@v5` tags. Add
persist-credentials: false to the actions/checkout configuration, while
preserving the existing Node.js version and npm cache settings.

Source: Linters/SAST tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant