fix(stream): make 1:1 recording possible, rate-limit Stream, lock down debug, give trials chat - #1138
Conversation
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 20 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds durable Stream webhook processing, recording eligibility for all appointment plans, circuit-breaker protection, bounded session reconciliation, maintenance channel recovery, Stream health reporting, authenticated debug access, rate limits, and scheduled orphaned-session reconciliation. ChangesStream integration hardening
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/cleanup/sweep-stuck-webhook-events.ts (1)
152-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSkip the Razorpay envelope reconstruction for Stream rows.
Lines 157-167 build
envelopefor every row, including Stream rows that never use it. Move the construction into theelsebranch so the Stream path does not pay for it and the code states which provider owns the envelope shape.🤖 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/cleanup/sweep-stuck-webhook-events.ts` around lines 152 - 167, Move the Razorpay envelope construction using payloadKeys and envelope into the non-Stream else branch of the loop over stuck events. Keep the Stream handling path free of this reconstruction, and preserve the existing RazorpayWebhookEnvelope shape for Razorpay events.
🤖 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 `@__tests__/enterprise/sweep-stuck-webhook-events.test.ts`:
- Around line 89-93: Add tests for the provider-specific redrive branches in the
sweep-stuck webhook events suite: mock `@/lib/stream/webhook-dispatch` and verify
a stream row calls processStreamEvent with its stored payload, eventType,
eventId, undefined signature, and extracted call_cid; also verify a Razorpay row
still calls processRazorpayWebhookEvent. Keep the existing provider query-filter
assertion unchanged.
In @.claude/skills/stream-sdk/SKILL.md:
- Around line 20-25: Specify a language on the fenced command block containing
the MCP, Supabase, and Netlify commands by changing its opening fence to use
text or shell, while leaving the command contents unchanged.
In @.github/workflows/reconcile-orphaned-sessions.yml:
- Around line 48-55: Update the reconcile workflow’s checkout and Node.js setup
steps to pin actions/checkout@v5 and actions/setup-node@v5 to immutable full
commit SHAs. Add job-level permissions granting only contents: read, and set
persist-credentials: false in the checkout step.
- Around line 60-64: Pin the workflow tools to project dependencies by adding
tsx@4.23.1 to package.json and package-lock.json, then update both the Prisma
client generation step and the orphaned session reconciliation step to use npx
--no-install. Preserve the existing commands and the reconciliation entry point.
In `@actions/maintenance/drain-sessions.ts`:
- Around line 181-195: Wrap the maintenance warning request in the same bounded
best-effort mechanism used for Stream operations, rather than relying on the
trailing catch after sendCallEvent settles. Update the sendCallEvent flow in the
session drain logic so a degraded API cannot delay call.end(), while preserving
the notification counter increment on successful delivery and the existing
withStreamCircuitBreaker call for ending the session.
- Around line 196-220: Update the session-draining flow around call.end() and
the subsequent prisma.$transaction so database completion is persisted only
after a successful end or a confirmed already-ended response. Detect
StreamUnavailableError and other unconfirmed failures, record the error without
marking the meeting session ended or the slot UNVERIFIED, and retry those
failures according to the existing retry mechanism.
- Around line 56-63: Update the activeSessions query in the drain flow to
exclude sessions whose appointment slot has not started by adding a startsAt <=
now condition to slotOfAppointment. Preserve the existing endsAt >= liveSince
and endedAt: null filters, and use the current time value consistently for the
startsAt comparison.
In `@app/api/meetings/`[meetingId]/join/route.ts:
- Around line 100-110: Update the join route’s error handling around
withStreamCircuitBreaker to import and detect StreamUnavailableError from
`@/lib/stream-client` before the generic catch. Log a warning and return the
existing retryable HTTP 503 response used for missing Stream configuration,
while preserving generic error handling for other failures.
- Around line 112-122: Update the admission flow surrounding the join route’s
membership check and updateCallMembers operation to enforce an attendee ceiling
or rate-limit/burst-handling strategy before admitting users. Preserve
membership-gated access, reject admissions beyond the configured plan or safe
Stream request capacity, and avoid issuing one unbounded updateCallMembers
request per attendee.
In `@app/api/stream/webhooks/route.ts`:
- Around line 140-154: Update the fallback event-key construction in the webhook
route to preserve every identity field’s position. Remove the `.filter(Boolean)`
call before `.join("_")`, keeping the existing empty-string placeholders and
direct join behavior so distinct field combinations cannot collapse to the same
key.
- Around line 158-165: Record the webhook durably before acknowledging it: in
app/api/stream/webhooks/route.ts lines 158-165, call logWebhookEvent("stream",
eventId, eventType, event, signature) before the after() handoff and 200
response. In lib/stream/webhook-dispatch.ts lines 200-208, move the
isDbHealthy() gate after recording the row or preserve a re-drivable row on
early return, and align both call sites’ claim semantics so processStreamEvent
does not treat the pre-ACK row as a duplicate.
- Around line 118-119: Update the webhook route’s JSON parsing and catch
handling around JSON.parse and the existing z.ZodError branch so malformed JSON
SyntaxError failures return the documented 400 response instead of falling
through to 500. Preserve the existing 400 behavior for validation errors and
ensure malformed deliveries stop retries.
- Around line 65-77: The webhook handling flow around getWebhookSecret must
decompress gzip-compressed Stream deliveries before signature verification and
JSON parsing, because Stream signs the uncompressed body. Update the route to
use the existing verifyAndParseWebhook helper if available, or otherwise detect
and decompress the request body before hashing and parsing while preserving
STREAM_API_SECRET and the X-Signature/X-Webhook-Id headers.
In `@app/meetings/`[id]/hooks/useGetCallById.ts:
- Around line 66-77: Update the non-OK response handling in the join-request
flow around setAccess to treat only HTTP 401 and 403 as access denials; route
HTTP 429 and all 5xx responses into the existing error state instead. Ensure the
error path does not leave access populated, since the page checks access before
error, and preserve the current response-message extraction for both outcomes.
- Around line 50-62: Update the request lifecycle in the hook’s run effect
around the join fetch to create an AbortController, pass its signal to fetch,
and call abort from cleanup when the component unmounts or callId changes.
Propagate the same cancellation through the subsequent callInstance.get()
operation if supported, while retaining the cancelled check first in the catch
block so AbortError is ignored before logging.
In `@lib/meetings/access.ts`:
- Around line 150-155: Update MeetingAccess and every return path in
resolveMeetingAccess to include a reason discriminant, using "not_found" for the
missing-meeting path. In app/api/meetings/[meetingId]/join/route.ts lines 81-94
and app/api/meetings/[meetingId]/validate-access/route.ts lines 44-58, select
the HTTP status using access.reason === "not_found" instead of comparing
access.message; the message should remain user-facing text only.
In `@lib/payments/webhooks/handlers.ts`:
- Around line 925-939: The direct-message channel must be provisioned durably
before payment or approval flows report success. In
lib/payments/webhooks/handlers.ts:925-939,
app/api/bookings/consultations/[consultationId]/route.ts:801-808, and
app/api/bookings/subscriptions/[subscriptionId]/route.ts:827-834, enqueue an
idempotent channel-provisioning command within each transaction instead of
provisioning only after success; ensure the command retries
createDirectMessageChannel until it succeeds, including redeliveries and
duplicate approvals.
In `@lib/stream/health.ts`:
- Around line 35-49: Update the health probe around
getStreamChatClient().getAppSettings() to use a short, SDK-supported request
timeout or AbortSignal that actually cancels the request. Ensure probe deadline
expiry is propagated through withStreamCircuitBreaker so it trips the breaker,
then preserve the catch response with reachable: false and breakerOpen
reflecting the resulting StreamUnavailableError.
In `@lib/stream/webhook-dispatch.ts`:
- Around line 43-59: Replace the separate HANDLED_EVENT_TYPES list and dispatch
switch with one event-type table keyed by event name, containing each event’s
schema and handler. Derive the route gate’s accepted types from that table and
use the same table for dispatch, including existing validation and default
handling behavior. Update the affected webhook routing logic around the
dispatcher so adding an event cannot leave the gate and dispatch out of sync.
- Around line 312-329: Update the inner error handling around the schema parse
cases and markWebhookEventProcessed so z.ZodError validation failures produce a
terminal error string prefixed with "gave up:". Preserve the existing retryable
error text for non-validation handler failures, allowing the sweeper to exclude
validation failures while continuing to retry transient errors.
In `@lib/webhooks/event-log.ts`:
- Around line 147-165: Remove the contradictory “processed=true only on success”
sentence from the docstring above markWebhookEventProcessed, while preserving
the explanation that failures record an error and leave processed=true for retry
detection.
- Around line 93-118: Update the stale-recovery and failed-retry branches in the
event-processing flow to atomically claim the row with conditional updateMany
calls. In the stale path, guard the claim with the existing event identity and
in-progress state; in the failed-retry path, also require error: { not: null }.
Treat a zero updated count as another worker owning the event and return the
existing non-new result, ensuring only the successful claimant returns isNew:
true.
In `@middleware.ts`:
- Around line 264-271: Update the “stream: api” limiter match predicate to
exclude the exact /api/stream/webhooks route while continuing to rate-limit
other /api/stream/ user-facing routes. Preserve the existing limiter
configuration and localhost behavior, and verify whether any additional
machine-to-machine endpoints under the prefix require the same exclusion before
finalizing the predicate.
- Around line 253-263: The meeting join flow currently limits by client IP
instead of the authenticated user. Update the join handler after
auth.api.getSession() to apply an Edge-compatible rate limiter keyed by
session.user.id, while preserving the existing unauthenticated handling and
streamJoinLimiter rule; do not rely solely on middleware or comment changes.
In `@scripts/cleanup/sweep-stuck-webhook-events.ts`:
- Around line 170-188: Make the terminal give-up handling provider-aware: when
the sweep stamps an event past giveUpAfterHours, use a Stream-specific reason
for ev.provider === "stream" instead of "gave up: payment never arrived", while
preserving the existing payment reason for other providers. Update the
deferred-event handling logic near the sweep’s terminal classification, not the
processStreamEvent dispatch branch.
---
Outside diff comments:
In `@scripts/cleanup/sweep-stuck-webhook-events.ts`:
- Around line 152-167: Move the Razorpay envelope construction using payloadKeys
and envelope into the non-Stream else branch of the loop over stuck events. Keep
the Stream handling path free of this reconstruction, and preserve the existing
RazorpayWebhookEnvelope shape for Razorpay events.
🪄 Autofix
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: 92737539-6555-4f01-86df-76e82592def1
📒 Files selected for processing (39)
.claude/skills/stream-sdk/SKILL.md.github/workflows/reconcile-orphaned-sessions.yml__tests__/enterprise/sweep-stuck-webhook-events.test.ts__tests__/security/dm-channel-org-precedence.test.ts__tests__/stream/recording-capability.test.ts__tests__/stream/stream-client.test.tsactions/maintenance/drain-sessions.tsapp/api/bookings/consultations/[consultationId]/route.tsapp/api/bookings/subscriptions/[subscriptionId]/route.tsapp/api/health/route.tsapp/api/meetings/[meetingId]/join/route.tsapp/api/meetings/[meetingId]/validate-access/route.tsapp/api/stream/debug/route.tsapp/api/stream/meetings/[streamCallId]/recording-info/route.tsapp/api/stream/recordings/start/route.tsapp/api/stream/recordings/stop/route.tsapp/api/stream/webhooks/route.tsapp/api/webhooks/utils.tsapp/meetings/[id]/hooks/useGetCallById.tsapp/meetings/[id]/page.tsxjobs/meetings/reconcile-orphaned-sessions.tslib/meetings/access.tslib/moderation/side-effects.tslib/payments/webhooks/handlers.tslib/rate-limit.tslib/stream-channel-ids.tslib/stream-client.tslib/stream-utils.tslib/stream/appointment-channels.tslib/stream/call-cid.tslib/stream/health.tslib/stream/recording-service.tslib/stream/recording-utils.tslib/stream/webhook-dispatch.tslib/webhooks/event-log.tsmiddleware.tsprisma/schema.prismascripts/cleanup/sweep-stuck-webhook-events.tsscripts/stream/ensure-call-type-grants.ts
6bd3ef6 to
82d9650
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
82d9650 to
33c9021
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
33c9021 to
352c5d8
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
Two findings routed to this PR from #1138's review — both about files this PR owns, and the first is made worse by the getOrCreate added in the last round. `useGetCallById` treated EVERY non-ok response from the join route as an access denial, defaulting the copy to "You are not authorized to join this meeting". So a 500, a 503 or a 400 told a legitimate participant they had been refused, with no retry affordance and no hint that anything was actually broken. Adding getOrCreate widened that: a Stream outage now reaches the client as a failure on a path that previously could not fail. Only 401/403/404 are verdicts now; everything else throws and gets the error UI. The join route returned 500 for a circuit-breaker trip. Stream being down is neither our fault nor the caller's, so it is a 503 with a retry message, which also keeps a provider outage out of the bucket that means "we broke something". `reason` stays absent on it — that field marks an authorization verdict, and an outage is not one, which is what lets the client tell them apart. The catch logs a hoisted meetingId rather than re-awaiting `params`, since re-awaiting would rethrow if `params` was itself what failed. Tests pin both: an outage is 503 with no `reason`, a genuine fault is still 500. The mocked error class is built inside the jest.mock factory — it is hoisted above every const in the file. tsc clean, eslint clean, 236 suites / 2680 tests pass. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
352c5d8 to
fbfc052
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
fbfc052 to
675a01f
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
675a01f to
e788bbe
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
…, permanent bans (#1136) * fix(stream): close the P0s — open call access, self-deleting channels, permanent bans (#1134) Six P0s from the #1134 audit, four of them confirmed against the live Stream app and the production database rather than by reading code. P0-1/P0-2 — any signed-in user could join any call. The `default` call type grants `join-call` to the plain `user` role, no token was call-scoped, and the only gate was a React conditional, so `client.call(type, id).join()` from devtools opened any consultation. Worse, `useGetCallById` ran `getOrCreate()` in parallel with the access check, so an unauthorized visitor minted a real billable call and became its `created_by` before seeing "Access Denied" — `default:smoke-test-nonexistent` and `default:test-meeting` were still sitting in production months later. - scripts/stream/ensure-call-type-grants.ts moves `join-call` off `user` onto `call_member` (dry-run by default, reversible with --restore-user-join). Hardens `default` in place because a call's type is immutable, so a new type would protect only future calls. - POST /api/meetings/[id]/join is now the sole grantor of membership, and grants only after resolveMeetingAccess passes. Membership rather than a `call_cids` token: the video client is an app-wide singleton holding one user token, and the JS SDK has no per-call token on a shared client. - The client creates nothing. validate-access keeps its read-only probe role and shares the one resolver so the two can't drift. P0-3 — `getDmChannelId` sorted with `localeCompare`, which orders by ICU collation and varies with build and locale. Commit 0116209 swapped `.sort()` for it and silently re-keyed most pairs; both variants were still live. Now code-unit ordering, pinned by tests using the real production ids that disagree. P0-7 — `consultation-<id>` / `subscription-<id>` channels deleted themselves. syncUserEventChannels expected only webinars, classes and DMs while treating both prefixes as MANAGED, so each was swept on the buyer's next dashboard load. Removed: the pair already gets a DM, and createConsultationChannel minted a DM anyway. Prefixes stay for legacy type resolution, out of MANAGED so survivors are left alone. P0-8 — all four createDirectMessageChannel call sites omitted organizationId, so an org-funded booking's DM landed on the personal `dm-` key while the reconciler expected `dmo-` — and then swept it. Threaded through, precedence matching bookingOrgId(). P0-4 — chat tokens were minted without `iat`. Stream treats an iat-less token as invalid once revocation is active, and that flag never clears, so a 7-day suspension was a permanent chat ban. Added, plus restoreStreamAccess() for lifting a deactivation. P0-5 — STREAM_WEBHOOK_SECRET is absent from Netlify and the route 500'd without it: 0 stream WebhookEvents, 0 MeetingAttendance, 0 of 1,663 sessions ended. Stream signs with the API secret, so that is now the default. Also adds lib/stream/call-cid.ts, one definition of the `type:id` split that four sites had each reinvented, and reports the payment-time channel provisioning failure to Sentry instead of claiming a nonexistent sync job will catch up. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): correct the grants fix against LIVE call-type data (#1134) Adversarial validation against the running Stream app found the P0 fix would have caused a total video outage, and would not have closed the hole it claimed to. Both verified by reading the live `default` call type, not by reasoning. The grants map has exactly six role keys: guest, user, call_member, admin, global_read_only, global_admin. There is NO `host` key and no `moderator` key. 1. LOCKOUT. The join route assigned `role: "host"` to consultants and `"user"` to everyone else. `host` is not a key in the grants map, so it confers nothing; `user` was about to lose `join-call`. Applying the script as written would have refused BOTH sides of every 1:1. Now always `call_member`, which is a strict superset of `user` on the live type (34 perms vs 33). Nothing is lost: host-ness in the UI comes from `custom.consultantUserId` via useCallCustomData(), never from the Stream role. The script now refuses to write a config where call_member lacks join-call. 2. GUEST. `guest` also holds `join-call`, and the app has `guest_user_creation_disabled: false` — guest sessions are creatable client-side with nothing but the public API key, which we ship as NEXT_PUBLIC_STREAM_API_KEY. Stripping only `user` would have left the devtools bypass fully intact behind a fix advertised as closing it. 3. `user` also holds `end-call`, `start-recording` and `stop-recording`. Any participant could end a call for everyone or start a recording directly from the client SDK — which walks straight around the consent gate in /api/stream/recordings/start, since that gate only guards our own endpoint. Stripped in the same pass. The old PRIVILEGED_ROLES loop was inert: `moderator`/`host` are not keys so the guard skipped them, and `admin`/`call_member` already held join-call. It read as protection and provided none. Removed. Also corrects the comment in lib/meeting.ts asserting that "the default call type does not restrict entry to members ... so naming members cannot turn a working join into a refusal". True when written; the exact opposite after this change, and precisely the comment someone would read while debugging a lockout. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): address PR review — heal call-less meetings, share the org resolver Triage of the 11 inline comments plus the outside-diff finding on #1136. Ten were real, one was wrong, and three were understated in ways that only showed up against the live Stream app. Verified live, because the docs disagree: the `default` call type's grants map has exactly six role keys — admin, call_member, global_admin, global_read_only, guest, user. There is no `host` and no `moderator`, and it is spelled `call_member`. Stream's published docs list five built-in roles including both of those and spell it `call-member`, which is what led review to suggest initialising grant entries for roles this app never assigns. No change made there. Three findings review understated: A MeetingSession row does not imply its Stream call exists. The seeds write rows with faker ids and no Stream object, `createDbMeetingSession` is a "use server" action validating the id as only `z.string().min(1)`, and maintenance drain ends the call while keeping the row. lib/meeting.ts skips its own getOrCreate whenever a row exists, and P0-2 removed the client-side one that used to paper over it — so resolveMeetingAccess granted access and updateCallMembers then 500'd. The join route now calls getOrCreate after the access check. Creating after authorization is the ordering P0-2 was about; creating before it was the vulnerability. The comment in lib/stream-utils.ts pointed at __tests__/stream/types.test.ts as the guard against localeCompare ordering. That test never imported getDmChannelId — it declared a local lambda that sorted with localeCompare and asserted that, so it would have passed straight through the regression it read like a guard against. It calls the real function now. The DM org divergence was in the query, not the precedence. All eight sites agreed on plan-then-appointment; the creator filtered `appointments` to org- tagged rows while consumers read an unordered `[0]`, so a mixed subscription got `dmo-…` from one and `dm-…` from the other. bookingOrgId moves to lib/stream-utils.ts, uses `find` rather than `[0]`, and the three `take: 1` reads gain the filter — truncation happens server-side, before `find` can run. Also found while verifying, not raised in review: call_member held start-recording and stop-recording, and the join route assigns call_member to every participant — so revoking them from `user` alone changed nothing while reading as a fix, and left the pre-join consent gate bypassable. Recording is server-only here (no client-side call.startRecording exists), so they are revoked from call_member too. `end-call` is deliberately kept: EndCallButton calls call.endCall() client-side and the Stream role no longer separates host from participant. Tracked separately rather than half-fixed. The refuse-guard could never fire — the transform adds join-call to call_member a few lines above the check, making the condition false by construction. It now runs against the post-apply re-read, where it can genuinely fail. That is the second zero-branch safety net in this file. updateCallType has undocumented merge-vs-replace semantics and the chat twin is a full replace, so the script snapshots settings and notification_settings, re- reads after applying, and writes a recoverable pre-image to disk on any drift. Re-sending them would require casting CallSettingsResponse into CallSettingsRequest, which could corrupt the config on its own. Remaining: MeetingAccess carries a `reason` discriminator so neither route infers its status from message text; the class/webinar participant check is an existence query rather than a slot fan-out; restoreStreamAccess rethrows anything that is not the already-active response; the consultation-approval catch reports to Sentry like its subscription twin; test fixtures are synthetic rather than real production user ids. tsc clean (cold), eslint clean (the two remaining warnings are byte-identical at HEAD), 236 suites / 2677 tests pass. Dry-run verified against the live app. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): second review round — org divergence in the webhook, healthy-path noise Four follow-up comments on #1136, all legit, all owned by this PR. The webhook handler was still resolving the DM org from the appointment being paid for. A subscription carries many appointments and is funded once, so for a mixed subscription that row can be the personal one — while createSubscriptionChannel resolves the first ORG-tagged row and mints `dmo-…`. Same pair, two channel ids: exactly the divergence this PR set out to close, surviving in the one site converted by hand. The query now loads an org-tagged subscription appointment and passes it to bookingOrgId, filtered in the query because `take: 1` truncates before `find` can choose. restoreStreamAccess reported the expected already-active response to Sentry before checking whether it was expected, so a lifted suspension — the healthy path — paged every time. Checked first now; only unexpected failures are reported, and those still throw. The settings-drift comparison used raw JSON.stringify over two independent getCallType reads, where key order is not guaranteed. A false positive there tells the operator Stream discarded a config it never touched, which is an expensive thing to be wrong about. Comparison is canonical now, sorting by code unit rather than localeCompare. The drift test drove the pre-image write into a real file in tmpdir on every run and left the payload unasserted — the payload being the only copy of the config Stream would have discarded. node:fs is mocked and the pre-image is asserted, plus a case pinning that key-order-only differences do not report drift. tsc clean, eslint clean, 236 suites / 2678 tests pass. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): a Stream outage is 503, and is not an access denial Two findings routed to this PR from #1138's review — both about files this PR owns, and the first is made worse by the getOrCreate added in the last round. `useGetCallById` treated EVERY non-ok response from the join route as an access denial, defaulting the copy to "You are not authorized to join this meeting". So a 500, a 503 or a 400 told a legitimate participant they had been refused, with no retry affordance and no hint that anything was actually broken. Adding getOrCreate widened that: a Stream outage now reaches the client as a failure on a path that previously could not fail. Only 401/403/404 are verdicts now; everything else throws and gets the error UI. The join route returned 500 for a circuit-breaker trip. Stream being down is neither our fault nor the caller's, so it is a 503 with a retry message, which also keeps a provider outage out of the bucket that means "we broke something". `reason` stays absent on it — that field marks an authorization verdict, and an outage is not one, which is what lets the client tell them apart. The catch logs a hoisted meetingId rather than re-awaiting `params`, since re-awaiting would rethrow if `params` was itself what failed. Tests pin both: an outage is 503 with no `reason`, a genuine fault is still 500. The mocked error class is built inside the jest.mock factory — it is hoisted above every const in the file. tsc clean, eslint clean, 236 suites / 2680 tests pass. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): make the org-tagged appointment choice deterministic, not just filtered Follow-up to the shared resolver. Filtering `appointments` to org-tagged rows fixed the case where a caller saw a personal row; it did not fix the case where two callers see DIFFERENT org-tagged rows. `take: 1` over a result with no `orderBy` — and `find` over one — is whatever Postgres returns, so a subscription carrying two org-tagged appointments could still resolve two different orgs across the creator, the webhook, the reconciler and the search route. Same divergence this PR set out to close, one layer down. Every appointment read that feeds bookingOrgId now orders by `[{ createdAt: "asc" }, { id: "asc" }]` — createdAt alone is not enough, because appointments created in one transaction share a timestamp. Eight sites: the five filtered `take: 1` reads and the three transaction-scoped reads in the subscription approval route that load the full list. Note this is the deterministic-resolver half of the review's suggestion. The structural half — storing the organization on `Subscription` so there is one canonical answer rather than a convention every caller has to honour — is a schema change and is tracked in #1144 rather than bolted on here. Pinned by a test asserting the ordering is present at every site, alongside the existing filter assertions. tsc clean, eslint clean, 236 suites / 2685 tests pass. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
e788bbe to
5039d86
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
5039d86 to
049ca62
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
049ca62 to
8f8fe8a
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@__tests__/stream/webhook-durability.test.ts`:
- Around line 59-65: Add a durability test covering the pre-ack claim collision
between recordStreamEventReceipt and processStreamEvent: configure
mockLogWebhookEvent to return isNew true for the receipt and false for the
subsequent webhook claim, then assert the registered handler still runs. Keep
the existing default stubs for unrelated tests and verify the ordering contract
through the relevant process flow.
In `@actions/maintenance/drain-sessions.ts`:
- Line 247: Track a durable frozen-channel set for each maintenance window and
always reverse exactly that set: in
actions/maintenance/drain-sessions.ts:247-247, freeze only channels whose
session completion is persisted or persist every frozen channel; in
actions/maintenance/drain-sessions.ts:357-364, load that window’s durable set
instead of discovering channels via the six-hour window or MAX_DRAIN_BATCH; in
app/api/admin/maintenance/route.ts:357-373, execute channel recovery
independently of runPostRecovery() failures so an OFF transition always
unfreezes channels.
- Around line 227-236: Update drainActiveSessions() to catch failures from the
per-session prisma.$transaction after call.end() succeeds, append the caught
error to result.errors, and continue processing subsequent active sessions
instead of rejecting the batch. Preserve reconciliation behavior for the
already-ended call row.
In `@lib/stream/recording-service.ts`:
- Around line 132-138: Update getCallRecordingsFromStream to rethrow
StreamUnavailableError instead of converting open-breaker failures to an empty
array, and update both per-session sync-loop catch blocks to propagate that
error rather than returning { synced: 0 }; preserve existing handling for other
errors.
In `@lib/stream/webhook-dispatch.ts`:
- Around line 242-260: The duplicate WebhookEvent claim prevents first-time
Stream events from reaching their handler. In
lib/stream/webhook-dispatch.ts:242-260, update processStreamEvent to accept or
recognize the receipt claim already made by the caller and continue dispatching
instead of returning on that matching in-progress row; in
app/api/stream/webhooks/route.ts:177-194, align recordStreamEventReceipt with
the dispatch claim so the pre-ack receipt is not treated as another worker’s
claim; in __tests__/stream/webhook-durability.test.ts:59-65, replace the
unconditional logWebhookEvent stub with coverage for an already-recorded second
claim and assert the handler still runs.
In `@lib/webhooks/event-log.ts`:
- Around line 48-147: Reduce cognitive complexity in logWebhookEvent by
extracting the failed-retry and stale in-progress claim logic into a shared
helper such as claimExistingEvent(eventId, where, payload). Have the helper
perform the conditional updateMany, return whether the row was claimed along
with the event record identifier as needed, and preserve the existing state
resets, payload/receivedAt updates, and count-based race handling. Replace both
branches in logWebhookEvent with their appropriate where clauses and logging
while keeping current retry and skip behavior unchanged.
🪄 Autofix
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: a7c53d72-11b3-4e20-b16d-30f35a631215
📒 Files selected for processing (10)
.github/workflows/reconcile-orphaned-sessions.yml__tests__/stream/webhook-durability.test.ts__tests__/stream/webhook-event-log-claim.test.tsactions/maintenance/drain-sessions.tsapp/api/admin/maintenance/route.tsapp/api/stream/webhooks/route.tslib/payments/webhooks/handlers.tslib/stream/recording-service.tslib/stream/webhook-dispatch.tslib/webhooks/event-log.ts
… chat (#1134) P1-6 — recording a 1:1 was not disabled, it was impossible. isAppointmentOwner and isRecordingEnabledForAppointment each hand-rolled an if/else over webinar and class only, so for a consultation or subscription both fell through to false: the consultant who OWNED the session failed the ownership check and start-recording answered 403 to them, while recording-info reported recordingEnabled:false regardless, above a comment noting the 1:1 plans had no such field. They have it now. ConsultationPlan and SubscriptionPlan gain recordingEnabled + recordingStoragePolicy mirroring WebinarPlan/ClassPlan, defaulting OFF — a 1:1 is the most sensitive session type here, so recording stays an explicit per-plan opt-in. Both predicates now read one resolveAppointmentPlan(), so they cannot disagree about which plan they are looking at, which was the shape of the original bug. The three routes that gate on them had to learn to fetch the 1:1 relations too; recording-info already resolved them for the consultant id but not for the flag. P1-11 — no Stream route or server action had ANY rate limit; neither /api/stream/ nor /api/meetings/ appeared in RATE_LIMIT_RULES. Two budgets: streamJoinLimiter (20/min) on the meeting join gate, which is the enumeration surface now that call ids are deterministic `slot-<anchorSlotId>`; and streamApiLimiter (60/min) on /api/stream/*, where every unbounded request was also a billable Stream API call. P1-12 — /api/stream/debug had no session check at all. Its only production gate was a shared secret in the QUERY STRING — which lands in access logs, browser history and Referer headers — and it dumps an arbitrary user's full Stream channel list. Now requires a session and staff/admin, with the secret demoted to defence in depth. Also fails closed when STREAM_DEBUG_SECRET is unset. P1-16 — TRIAL had no branch in channel provisioning and "trial" was not even in eventTypeSchema, so a trial buyer got video and no way to message the consultant. A trial is the platform's first impression and was the one session type that shipped mute. It gets the same DM as any other 1:1, so the thread merges if they go on to book. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
tsc rejected the `unknown` plan param — I ran the typecheck before adding this file and only ran jest afterwards, so CI would have caught what I did not. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
8f8fe8a to
5f84d53
Compare
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
…eliveries The `stream: api` rule matched `/api/stream/` by prefix, which swept in `/api/stream/webhooks`. That is worse than an ordinary throttle. Stream POSTs every delivery from its own infrastructure, so all of them collapse onto one rate-limit key rather than spreading across users, and bursts are the normal shape — a 200-attendee webinar emits 200 `call.session_participant_joined` events at once. A 429 is not a deferral: Stream retries 408/429/5xx inside a fifteen-second total budget and then drops the event permanently. So this would have silently reintroduced exactly the loss #1137's persist-before-ack work exists to prevent, and done it in the middleware, before the route ran, where none of that machinery applies. One PR making webhooks durable and the next making them droppable. Excluding the path is safe because the endpoint is not open: it verifies an HMAC signature against the API secret and 401s anything unsigned before doing any work. The signature is the gate; the limiter never was. Separately, the join rule's comment claimed it was "keyed per user by the shared resolver". It is not. `applyEdgeRateLimits` falls back to the client IP whenever a rule supplies no `key`, and this rule supplies none — per-user keying is not available here by design, since the middleware is cookie-presence only with no DB hit and no JWT parsing. IP-keying is the right shape for enumeration anyway, because a walker works from one address; the cost is that shared-NAT users share a bucket, which is why the limit is generous. The comment now says so. Pinned by a test that asserts the rule table directly rather than booting the middleware, since the matcher predicates are the whole behaviour and the middleware pulls in the edge runtime, Redis and the maintenance store. tsc clean, eslint clean (the one remaining warning is pre-existing on dev), 240 suites / 2717 tests. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
e02df9a to
a3d45d3
Compare
|
❌ The last analysis has failed. |
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
… works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
… works (#1139) * feat(stream): pre-join recording consent with a refusal that actually works (#1134 P1-7) Before this, a consultee's first sign that a session was being recorded was a small `REC hh:mm` pill that appeared AFTER recording had already started. No pre-join notice, no way to refuse, and no record that anyone had been told — on a platform whose 1:1 sessions are career and health conversations. #1138 makes 1:1 recording possible for the first time, which makes this urgent rather than theoretical, and is why that flag defaults off. Two regimes, because one rule cannot be honest for both: 1:1 (consultation / subscription / trial) — a real opt-out. Declining costs nothing: they still join, and POST /api/stream/recordings/start returns 409. If a refusal has no effect it is not consent, it is a notice wearing a consent costume. Group (webinar / class) — notice and acknowledgement. The recording IS the product: attendees buy the replay and it was disclosed at purchase. One attendee cannot veto what 199 others paid for, so the API refuses to write DECLINED for a group session and the copy says plainly that opting out means cancelling for a refund. The gate also ignores any DECLINED row that reaches a group session by another route, so a legacy or hand-written row cannot kill a replay. New `MeetingRecordingConsent`, unique on (meetingSession, user). Its own model rather than columns on MeetingAttendance: those rows are written by Stream's participant webhooks at join time, while consent is a lobby decision taken BEFORE joining, and a compliance record must not depend on a webhook having fired. Not ConsentArtifact either — that is account-level with no session key. `noticeVersion` records which wording was shown, so a later copy change cannot retroactively reinterpret an old decision. The prompt renders in the existing lobby and disables Join until answered, so nobody reaches the call without having seen it — and only when the plan has recording enabled, so it stays off the overwhelming majority of sessions. The notice fetch fails OPEN: a lobby must never trap someone because a disclosure endpoint 500'd, and recording is separately gated server-side, so failing open cannot produce an unconsented recording. Two deliberate details. Decline and Allow carry equal visual weight — making the refusal quieter is how a consent prompt becomes a dark pattern. And the host-facing block reason never names who declined, because telling a consultant which participant refused makes refusing socially costly, which is the same as not offering the choice. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * docs(stream): say plainly that consent enforcement is start-time only Review found that a refusal has no effect once a recording is already running, in the PR whose title is "a refusal that actually works". The finding is right, and the gap is reachable rather than theoretical: `recordRecordingConsent` upserts, so a participant who granted in the lobby can switch to DECLINED at any point, and a late joiner can decline while a recording the host started earlier keeps going. In the OPT_OUT regime that leaves the refusal inert for the rest of the session. Closing it means the decline path reading `MeetingSession.isRecording` and, when a recording is live, stopping it through the same route the stop endpoint uses and clearing the recording columns. That is a product decision rather than a refactor. It hands any participant the ability to terminate a host's in-progress recording mid-call, and getting it wrong costs a consultant a session they believed was being recorded. Guessing at it unattended is worse than tracking it. So this takes the reviewer's own stated alternative: record the gap where someone will actually read it, rather than let the contract imply enforcement the code does not provide. The SCOPE note sits on `getRecordingBlock`, which is what the start route consults, and the decline write path now says what it does not do. No behaviour change. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): make the decline check part of the recording claim, not a read before it Two review findings on the consent gate, one a real race and one a scope limit worth stating loudly. The race: `getRecordingBlock` counts DECLINED rows, and the start route then claims `isRecording` in a separate `updateMany`. A participant committing a decline in the window between them meant recording started despite a refusal already in the database — check-then-act with a compliance record as the loser, the same shape as the webhook claim fixed in #1137. The consent condition is now part of the claim itself, as a relation filter that Postgres evaluates at write time, so a decline that commits first makes the update match zero rows. No transaction needed. The earlier read stays because it produces the specific user-facing reason; the predicate on the write is the part that has to be true. On a zero-row result the route re-reads to distinguish "a decline landed" from "already recording", so a refused host is not told the wrong thing. The scope limit: this gate guards an HTTP route, not Stream. On the LIVE call type `call_member` — the role every participant gets at join — still holds `start-recording` and `stop-recording`, so `call.startRecording()` from devtools walks straight past the 409. #1136 ships the script that strips both, but merging #1136 does not RUN it; it is an operator action still outstanding. Until it is applied, this feature is advisory, and the module now says so where someone will read it rather than leaving the contract to imply enforcement that is not switched on yet. tsc clean, eslint clean, 243 suites / 2742 tests. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * refactor(stream): one definition for the recording-refusal response SonarCloud failed #1139 on new-code duplication at 4.5% against a 3% threshold — the only condition not passing. The cause was mine: the race fix added a second `getRecordingBlock` check on the zero-row path whose refusal response was near-identical to the pre-claim gate's. Both now go through one `refuse()` helper, which also means the "recording refused" log line has a single call site rather than one branch logging and the other silently returning. Note for anyone reading the Sonar report: it also lists a cross-file duplication between this route's preamble (lines 22-45) and stop/route.ts. That one is pre-existing, untouched by this PR, and is the shared auth-and-parse opening the two recording routes have always had. Worth extracting eventually, but not here — it is not what failed the gate. tsc clean, eslint clean. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Four P1s from #1134. Three are security or compliance; one is a product capability that was structurally absent.
Recording a 1:1 was impossible, not disabled
isAppointmentOwnerandisRecordingEnabledForAppointmenteach hand-rolled an if/else overwebinarandclass. For a consultation or subscription, both fell through tofalse. So:POST /api/stream/recordings/startanswered 403 to them;recording-inforeportedrecordingEnabled: falseregardless, above a comment reading "Consultations and subscriptions don't have recordingEnabled on their plans" — accurate, and the reason the whole path was dead.Fixed properly rather than patched:
ConsultationPlanandSubscriptionPlangainrecordingEnabled+recordingStoragePolicy, mirroringWebinarPlan/ClassPlanexactly. Defaultfalse— a 1:1 is the most sensitive session type on the platform, so recording stays an explicit per-plan opt-in by the consultant, never implicit.resolveAppointmentPlan()returns the plan behind an appointment whichever of the four kinds it is, and both predicates read it. They can no longer disagree about which plan they are looking at — which is the shape of the original bug.recordings/start,recordings/stop,recording-info) had to learn to fetch the 1:1 relations.recording-infoalready resolved them for the consultant id but not for the flag, which is exactly how the two halves drifted.Zero rate limiting on anything Stream
Neither
/api/stream/nor/api/meetings/appeared inRATE_LIMIT_RULES. Two budgets, sized to the two shapes:streamJoinLimiterslot-<anchorSlotId>, so this is the enumeration surface — anyone holding one slot id could walk neighbours probing which meetings they can reach. Loose enough that a flaky network retrying a join never trips it.streamApiLimiter/api/stream/*— ordinary authenticated reads and writes, but every unbounded request was also a billable Stream API call./api/stream/debughad no session check at allIts only production gate was a shared secret in the query string — which lands in access logs, browser history and
Refererheaders — and it dumps an arbitrary user's full Stream channel list. Now requires a session and staff/admin, with the secret demoted to defence in depth. It also fails closed whenSTREAM_DEBUG_SECRETis unset; the old comparison would have matchedundefinedagainstundefined.Trial buyers had no chat
TRIALhad no branch in channel provisioning, and"trial"was not even ineventTypeSchema, so it could not be requested. A trial buyer got video and no way to message the consultant before or after it — the platform's first impression was the one session type that shipped mute. It now gets the same DM as any other 1:1, so the thread merges if they go on to book.Verification
tsc --noEmitclean,prisma validateclean.eslintclean on every changed file. (middleware.ts:351 eqeqeqis pre-existing ondev— confirmed by stashing — and== nullis deliberate there, so I left it rather than widen a security PR.)__tests__/stream/recording-capability.test.tsthat assert all four appointment kinds behave identically, that an absent flag defaults closed, that a nullconsultantProfileIdnever matches a null caller, and that the two predicates always resolve the same plan.Needs a
db pushTwo nullable-with-default columns on each of
ConsultationPlanandSubscriptionPlan. Additive only; no renames, no drops, no type changes.Follow-up, deliberately not here
Recording consent (#1134 P1-7) is still open. The consultee currently sees a passive
RECpill only after recording starts — no pre-join disclosure, no opt-out, no consent record. That is a DPDP exposure on a consultation product and it needs a UI flow plus a consent record, which does not belong bolted onto this diff. Enabling 1:1 recording makes it more urgent, not less, which is why the default isfalse.Part of #1134
🤖 Generated with Claude Code
https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
Summary by CodeRabbit
New Features
Improvements
Security