fix(stream): subscribe the missing webhook events, unfreeze chat after maintenance - #1141
Merged
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: 108 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 (3)
📝 WalkthroughWalkthroughThe PR centralizes Stream meeting access, recording consent, webhook processing, channel lifecycle management, batching, provisioning, health reporting, and operational automation. It also adds supporting tests, rate limits, token handling, and Stream integration documentation. ChangesStream foundations
Meeting access and recording
Webhook and lifecycle operations
Stream operations and security
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 38
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
scripts/cleanup/sweep-stuck-webhook-events.ts (1)
196-214: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe give-up message is Razorpay-specific but Stream rows now reach it.
processStreamEventreturns early when its DB-health gate trips, which leaves the row atprocessed=false, error=null. That is the defer signature this branch matches. A Stream row aged pastgiveUpAfterHoursis then stampedgave up: payment never arrivedand that text is stored, pushed intoerrors, and logged. Stream events carry no payment, so on-call reads a wrong cause.Make the message provider-aware.
🔧 Proposed fix for the give-up message
if (ev.receivedAt < giveUpOlderThan) { + const giveUpReason = + ev.provider === "stream" + ? "gave up: event still deferred" + : "gave up: payment never arrived"; await prisma.webhookEvent .update({ where: { eventId: ev.eventId }, data: { processed: true, - error: "gave up: payment never arrived", + error: giveUpReason, }, }) .catch(() => {}); gaveUp++; - errors.push(`${ev.eventId}: gave up: payment never arrived`); + errors.push(`${ev.eventId}: ${giveUpReason}`);The
NOT: { error: { startsWith: "gave up:" } }guard at Line 127 still matches both messages.🤖 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 196 - 214, Make the terminal give-up message in the deferred-event branch provider-aware instead of always using the Razorpay-specific “payment never arrived” text. Use the event’s provider to select the appropriate message, and apply that same selected message consistently to the database error, errors entry, and warning log while preserving the existing give-up guard behavior.jobs/meetings/reconcile-orphaned-sessions.ts (1)
99-126: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDistinguish a Stream outage from a missing call before you finalize the session.
The
catchat Line 114 treats every failure the same way. It stampsendedReason: "stream_not_found"and setscompletionStatus: "UNVERIFIED"with the slot end time. OnceendedAtis set, the session no longer matches the orphan query, so the realended_atfrom Stream is never recovered.The circuit breaker changes the blast radius. Before this change, an outage produced a 30s timeout per session and the run usually died partway. Now
withStreamCircuitBreakerthrowsStreamUnavailableErrorimmediately, so a single run finalizes all 100 rows as UNVERIFIED within seconds. Fast-failing is correct; finalizing on a fast failure is not.Skip the session when the breaker is open, and let the next run retry it.
🔧 Proposed fix
} catch (streamError) { + // A breaker-open outage is not evidence about the call. Leave the + // session orphaned so a later run can ask Stream again. + if (streamError instanceof StreamUnavailableError) { + result.details.push( + `Session ${session.id}: skipped, Stream unavailable`, + ); + continue; + } // Stream API error or call not found — use slot end time endedAt = new Date(session.slotOfAppointment.endsAt);Import
StreamUnavailableErrorfrom../../lib/stream-clientalongside the existing imports.🤖 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 `@jobs/meetings/reconcile-orphaned-sessions.ts` around lines 99 - 126, Update the catch handling in the orphan reconciliation flow to detect StreamUnavailableError separately from missing-call failures. Import StreamUnavailableError from ../../lib/stream-client, and when caught, skip finalizing the current session so its orphan status remains retryable; preserve the existing slot-end, "stream_not_found", counter, and warning behavior for other errors.actions/stream/chat/user.action.ts (1)
224-240: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMerge the per-batch
usersmaps before returning.
resultis overwritten for each chunk, so rosters above 100 return only the final batch.upsertUsersreturnsusers: Record<string, UserResponse>in stream-chat 9.30.1. Accumulate intoAwaited<ReturnType<typeof client.upsertUsers>>["users"]and return{ users: mergedUsers }to preserve the non-optional return contract.🤖 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 `@actions/stream/chat/user.action.ts` around lines 224 - 240, Merge each batch’s result.users map in the forEachChunk flow instead of overwriting result, using the users type from Awaited<ReturnType<typeof client.upsertUsers>>. After processing all batches, return an object containing the merged users map while preserving the existing sync behavior and non-optional return contract.app/meetings/[id]/page.tsx (1)
68-79: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winServer and configuration failures render as "Access Denied".
useGetCallByIdmaps every non-ok response fromPOST /api/meetings/[meetingId]/jointo{ hasAccess: false, message }, including 401, 500, and the 503 "Video is not available" case. This branch then shows those messages under an "Access Denied" heading with a shield icon and the text "If you believe this is an error, contact support or the meeting host". A user hitting a Stream outage is told they lack permission.Fix the root cause in
app/meetings/[id]/hooks/useGetCallById.tsLines 66-77: treat only 401 and 403 as an access refusal, and route 5xx toerror.🛠️ Proposed fix in the hook
if (!response.ok) { const body = await response.json().catch(() => ({})); const message = typeof body?.error === "string" ? body.error : "You are not authorized to join this meeting"; + // Only an authorization verdict is an access refusal. A 500 or a 503 + // is a failure, and showing it under "Access Denied" tells the user + // the wrong thing about a Stream outage. + if (response.status >= 500) { + setError(new Error(message)); + setCall(null); + return; + } setAccess({ hasAccess: false, role: null, message }); setCall(null); // Not an `error`: a refusal is an expected outcome with its own UI, // and rendering it as a crash lost the reason. return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/meetings/`[id]/page.tsx around lines 68 - 79, Update useGetCallById so only 401 and 403 responses from the join request produce hasAccess: false access-refusal results; route 5xx responses, including the 503 video-unavailable case, through error instead. Preserve the existing successful response handling and user-facing access-denied flow for genuine authorization failures.app/api/health/route.ts (1)
92-104: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the Stream health probe to 5 seconds.
getStreamStatus()can wait up to 30 seconds forgetAppSettings(), delaying the entire health response. Returnreachable: nullwhen a 5-second timeout expires. Abort the underlying request and clear the timer, becausePromise.racealone leaves the Stream request running.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/health/route.ts` around lines 92 - 104, Update the health flow around getStreamStatus() to enforce a 5-second timeout, returning reachable: null when the deadline expires. Abort the underlying Stream request on timeout and clear the timer when either the request completes or times out; do not use Promise.race alone without cancellation.
🤖 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 @.claude/skills/stream-sdk/SKILL.md:
- Around line 48-51: Update the webhook delivery guidance in the skill
documentation to specify three total Stream delivery attempts within 15 seconds,
including two retries for network errors; do not describe the delivery contract
as five attempts, and distinguish delivery retries from internal processing
retries while preserving the existing ack, idempotency, and signing guidance.
In @.github/workflows/expire-event-channels.yml:
- Around line 15-18: Update both .github/workflows/expire-event-channels.yml
(lines 15-18) and .github/workflows/reconcile-orphaned-sessions.yml (lines
26-29) with top-level contents: read permissions, and set persist-credentials:
false on each actions/checkout@v5 step. Also pin actions/checkout and
actions/setup-node references to commit hashes in both workflows.
- Around line 52-53: Update the workflow step name associated with the
expire-event-channels.ts command so it accurately describes the expire event
channels job instead of release earnings; leave the command unchanged.
- Around line 29-31: Align the fail-closed comment in the workflow with the
actual lock behavior: update the `withCronLock` call in
`expire-event-channels.ts` to use fail-closed mode so missing Redis
configuration prevents execution, or revise the workflow comment if fail-open
behavior is intentional. Ensure the comment and `failMode` value consistently
describe the chosen behavior.
In `@actions/maintenance/drain-sessions.ts`:
- Around line 331-338: Update unfreezeChannelsAfterMaintenance to avoid using
LIVE_SESSION_WINDOW_MS as the maintenance lookback; instead, scope the query to
the recorded MaintenanceWindow start time or use a dedicated sufficiently wide
maintenance-window constant. Also remove or adjust the MAX_DRAIN_BATCH
limitation and ensure the query selects every maintenance-ended session frozen
by the drain, regardless of ordering.
- Around line 296-309: Update the freeze operation around
withStreamCircuitBreaker and Promise.allSettled so each channel rejection is
handled like the existing unfreeze path: record the failed channel and error in
result.errors while preserving successful updates. Ensure the circuit breaker
still receives a rejection when freeze failures occur, rather than allowing
Promise.allSettled to hide all failures.
In `@app/api/admin/maintenance/route.ts`:
- Around line 363-368: Update the unfreeze handling around
unfreezeChannelsAfterMaintenance to call Sentry.captureException for thrown
failures using the file’s established subsystem tag, while retaining the error
response behavior. After a successful call, inspect chat.errors and report any
populated per-channel failures to Sentry as well, and ensure the response
surfaces those partial failures.
In `@app/api/bookings/subscriptions/`[subscriptionId]/route.ts:
- Around line 823-826: Make the organization fallback deterministic in the
subscription handling flow around the dmOrgId assignment: add the same total
order to the approval-include query and both getDmPairsForUser queries before
selecting appointments, ensuring appointments[0] consistently represents the
same organization across all three paths.
In `@app/api/meetings/`[meetingId]/join/route.ts:
- Around line 110-120: Apply applyRateLimit(streamJoinLimiter, session.user.id)
immediately after authentication and before the meeting access check or any
updateCallMembers write in the join route. Preserve the existing access
validation and withStreamCircuitBreaker flow, while ensuring repeated
authenticated join attempts are capped by the existing limiter.
In `@app/api/meetings/`[meetingId]/recording-consent/route.ts:
- Around line 105-110: Add a stable discriminant such as reason: "not_found" |
"forbidden" to MeetingAccess in the access logic, populate it for missing and
unauthorized meetings, and update both access.hasAccess failure handlers in the
recording-consent route to derive 404 versus 403 from access.reason rather than
access.message; preserve access.message solely as the response error text.
- Around line 32-88: Remove the duplicate meeting lookup by reusing the record
loaded in resolveMeetingAccess. Return or pass through the session id and
existing appointment plan shape needed by loadAppointment, then update the POST
handler and loadAppointment flow to consume that record instead of querying
prisma.meetingSession again.
In `@app/api/stream/recordings/start/route.ts`:
- Around line 136-150: Make consent enforcement apply both at recording start
and after a later decline: in app/api/stream/recordings/start/route.ts:136-150
retain and document the pre-claim gate as the start-time half; in
app/api/meetings/[meetingId]/recording-consent/route.ts:204-208, after an
OPT_OUT session is marked DECLINED, stop any live recording through the existing
stop-route path and clear MeetingSession.isRecording, recordingStartedAt, and
recordingStartedBy; in lib/stream/recording-consent.ts:136-154 expose a shared
helper used by both paths so the blocking rule has one definition.
In `@app/meetings/`[id]/components/MeetingSetup.tsx:
- Line 167: Reduce MeetingSetup’s cognitive complexity by extracting the
handleJoinMeeting switch on call.state.callingState into a helper function
outside the MeetingSetup component. Pass the helper the required state,
callbacks, and meeting data, preserve each existing switch branch’s behavior,
and have handleJoinMeeting delegate to it.
- Around line 475-479: Update useRecordingConsent and its MeetingSetup consumer
to expose an explicit loading state while the consent notice request is pending.
Use that state to render a visible loading or pending indicator beside the Join
button and distinguish it from a completed unsatisfied consent result, so the
button is not silently disabled while consent.node is null.
In `@app/meetings/`[id]/components/RecordingConsentNotice.tsx:
- Around line 55-68: In the notice-loading effect, re-check the existing
cancelled flag after await res.json() and before setNotice so stale responses
cannot update state after meetingId changes or unmount. Preserve the existing
non-OK fallback and catch-path cancellation guards, and apply the guard
specifically to the Notice assignment path.
In `@app/meetings/`[id]/hooks/useGetCallById.ts:
- Around line 44-48: Update the client-wait logic in the useGetCallById effect
so a missing client remains loading initially but transitions to an error state
after a bounded timeout. Clear or cancel the timeout when client or callId
becomes available or the effect unmounts, and surface the existing error
state/message so page.tsx can render a failure instead of an endless skeleton.
- Around line 87-90: Update the call-loading flow around client.call() and
callInstance.get() to handle a missing Stream call before updateCallMembers is
invoked. Recover by creating the missing call when appropriate, or return a
dedicated missing-call error, ensuring the join request does not fail
unexpectedly while preserving normal existing-call behavior.
In `@jobs/stream/expire-event-channels.ts`:
- Around line 122-125: Update the !isStreamConfigured() branch in the
expire-event-channels function to set result.success = false before returning,
so missing Stream configuration is reported as a failed run. Preserve the
existing error collection and early-return behavior.
- Around line 89-93: Refactor expireEventChannelsUnlocked by extracting the
freeze iteration and delete iteration into named helpers, preserving their
current behavior and ordering. Replace the nested ternary that derives channelId
with a small resolveChannelId helper handling webinar, class, and null cases.
Keep expireEventChannelsUnlocked responsible only for orchestration while
retaining existing channel expiration behavior.
- Around line 63-79: Update loadEndedEvents to bound the appointment scan with a
lower endsAt threshold based on the maximum retention period plus a grace
margin, excluding events older than that window. Ensure the same bounded result
prevents already-deleted historical channels from reaching the toDelete handling
around the deletion logic, while preserving current retention behavior for
eligible events.
- Around line 209-219: Update the runJob callback in the main entrypoint so
prisma.$disconnect() executes in a finally block around expireEventChannels and
its result handling, including when expireEventChannels rejects; preserve the
existing logging and failure exit-code behavior.
In `@lib/meetings/access.ts`:
- Around line 150-155: Add a discriminated reason field to the MeetingAccess
result in lib/meetings/access.ts, using not_found, unauthorized, or granted, and
set it on every return path including the missing-meeting case. Update
app/api/meetings/[meetingId]/join/route.ts lines 90-93 and
app/api/meetings/[meetingId]/validate-access/route.ts lines 44-53 to determine
status from access.reason instead of matching the human-readable message; the
resolver file is the root change, while both route sites require direct updates.
In `@lib/moderation/side-effects.ts`:
- Around line 363-371: Update the reactivateUser call in restoreStreamAccess to
propagate failures instead of swallowing them through the broad catch and
captureModerationError path. Remove the catch, or restrict handling to a
documented error code indicating the user is already active; all timeouts, rate
limits, and server errors must cause restoreStreamAccess to reject.
In `@lib/payments/webhooks/handlers.ts`:
- Around line 925-939: Update the appointment query and consultant resolution in
the webhook handler to include trialSession.consultantProfile, then use that
profile when resolving consultantUserId for TRIAL events. Preserve the existing
consultation and subscription resolution paths and ensure trial appointments
without another appointment relation reach createDirectMessageChannel.
- Around line 905-939: Centralize organization resolution in a shared resolver
and use it across bookingOrgId, channel creators, the webhook handler, approval
routes, and reconciler, including createConsultationChannel and
createSubscriptionChannel. Preserve precedence: plan organization, consultation
appointment organization, first non-null subscription appointment organization,
then null. Update subscription queries and selection logic so all paths use the
first non-null appointment organization rather than appointments[0] when it is
null. Apply this to lib/payments/webhooks/handlers.ts lines 905-939,
app/api/bookings/consultations/[consultationId]/route.ts lines 791-809, and
app/api/bookings/subscriptions/[subscriptionId]/route.ts lines 823-826; the
remaining shared-resolver call sites are actions/stream/chat/channel.action.ts
and actions/stream/chat/event-channel.action.ts.
In `@lib/stream/recording-consent.ts`:
- Line 157: Update the re-export for resolveAppointmentPlan to use an
export-from declaration, and remove resolveAppointmentPlan from the file’s
import list if it is not referenced elsewhere in the module.
In `@lib/stream/webhook-dispatch.ts`:
- Around line 213-295: Make the dispatch flow around the event-type switch
exhaustive for HandledEventType: validate or narrow the incoming string at the
route boundary, pass the narrowed value into the dispatcher, and add an
unreachable/default exhaustiveness guard that causes compilation to fail when a
handled type lacks a case. Preserve explicit handling for all existing event
types and ensure unsupported strings retain the current unhandled-event
behavior.
- Around line 184-192: Remove the isDbHealthy health-probe block from the
webhook dispatch flow before logWebhookEvent, allowing database failures to
propagate to the existing outer catch and report the event-loss signal
accurately. Also remove the now-unused isDbHealthy import, while preserving the
existing acknowledgement and logging behavior.
In `@lib/webhooks/event-log.ts`:
- Around line 93-113: Track processing attempts separately from the original
receipt time: add a nullable attemptedAt field to WebhookEvent, initialize it to
the current time when creating a record, and refresh it in both reset paths.
Update the in-progress staleness calculation to compare Date.now() against
attemptedAt, preserving the existing retry behavior while preventing overlapping
sweeps or redeliveries from concurrently reprocessing the same event.
In `@middleware.ts`:
- Around line 253-263: Update the “stream: meeting join” rate-limit rule to
provide a key resolver that returns the authenticated user ID, reusing the
existing shared user-ID resolver used by other RateRule entries. Keep the rule’s
matcher, limiter, and localhost behavior unchanged so join attempts are budgeted
per authenticated user rather than by client IP.
- Around line 264-271: Update the “stream: api” rule’s match predicate to
exclude the `/api/stream/webhooks` endpoint while continuing to match other
`/api/stream/` routes. Leave webhook handling to its existing signature
verification and idempotent event-log protections.
In `@prisma/schema.prisma`:
- Around line 3991-4019: Apply the Prisma schema changes, including
MeetingRecordingConsent, RecordingConsentDecision, and the related plan columns,
with npm run db:push before deployment. Do not use npm run db:push:schema;
ensure the sidecar schema changes are applied before
lib/stream/recording-consent.ts runs.
In `@providers/StreamProviderImpl.tsx`:
- Around line 259-279: Update the syncUserEventChannels promise handling so only
a resolved result with success === true persists syncKey in sessionStorage and
retains clientSyncCompletedUsers. For unsuccessful results and rejected
promises, delete userDetails.id from clientSyncCompletedUsers and leave
sessionStorage unset; keep the existing logging behavior.
In `@scripts/stream/ensure-call-type-grants.ts`:
- Around line 118-126: Rename the loop-local boolean `before` in the
grant-reporting loop to a distinct name, and update its use in the `join-call`
log expression. Leave the outer `before` string and the existing grant
comparison behavior unchanged.
- Around line 147-148: Update the call-type flow around
client.video.updateCallType to first preserve the existing settings,
notification_settings, and external_storage values, then merge them with the new
grants in the PUT payload. Ensure --apply retains all existing call-type
configuration so --restore-user-join can restore it unchanged.
In `@scripts/stream/ensure-webhook-subscription.ts`:
- Around line 79-109: The loop currently updates the app-level event_hooks array
with only the current hook, replacing other hooks and prior changes. In the
hook-processing flow, accumulate each unchanged or updated hook in a complete
event_hooks collection, then call updateAppSettings once after the loop with
that full array; preserve existing event types while adding missing ones when
apply is enabled.
- Around line 62-67: In the app-settings retrieval in
ensure-webhook-subscription, remove the unchecked `as unknown as` cast from
`client.getAppSettings()` and rely on the SDK-inferred `AppSettingsAPIResponse`
type. Preserve the existing `event_hooks` filtering through
`app.app?.event_hooks` while allowing future response-shape changes to surface
as type errors.
In `@scripts/stream/stream-sync.ts`:
- Around line 160-165: Update the SyncOptions requireLock documentation to state
that its default is true, and revise the lock-acquisition catch warning to
remove the phrase “proceeding without distributed lock.” Preserve the existing
fail-closed behavior for the default path and leave explicit requireLock
handling unchanged.
---
Outside diff comments:
In `@actions/stream/chat/user.action.ts`:
- Around line 224-240: Merge each batch’s result.users map in the forEachChunk
flow instead of overwriting result, using the users type from
Awaited<ReturnType<typeof client.upsertUsers>>. After processing all batches,
return an object containing the merged users map while preserving the existing
sync behavior and non-optional return contract.
In `@app/api/health/route.ts`:
- Around line 92-104: Update the health flow around getStreamStatus() to enforce
a 5-second timeout, returning reachable: null when the deadline expires. Abort
the underlying Stream request on timeout and clear the timer when either the
request completes or times out; do not use Promise.race alone without
cancellation.
In `@app/meetings/`[id]/page.tsx:
- Around line 68-79: Update useGetCallById so only 401 and 403 responses from
the join request produce hasAccess: false access-refusal results; route 5xx
responses, including the 503 video-unavailable case, through error instead.
Preserve the existing successful response handling and user-facing access-denied
flow for genuine authorization failures.
In `@jobs/meetings/reconcile-orphaned-sessions.ts`:
- Around line 99-126: Update the catch handling in the orphan reconciliation
flow to detect StreamUnavailableError separately from missing-call failures.
Import StreamUnavailableError from ../../lib/stream-client, and when caught,
skip finalizing the current session so its orphan status remains retryable;
preserve the existing slot-end, "stream_not_found", counter, and warning
behavior for other errors.
In `@scripts/cleanup/sweep-stuck-webhook-events.ts`:
- Around line 196-214: Make the terminal give-up message in the deferred-event
branch provider-aware instead of always using the Razorpay-specific “payment
never arrived” text. Use the event’s provider to select the appropriate message,
and apply that same selected message consistently to the database error, errors
entry, and warning log while preserving the existing give-up guard behavior.
🪄 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: d6c9681f-6ed8-4ffc-9274-a74bc509280e
📒 Files selected for processing (57)
.claude/skills/stream-sdk/SKILL.md.github/workflows/expire-event-channels.yml.github/workflows/reconcile-orphaned-sessions.yml.github/workflows/stream-sync.yml__tests__/enterprise/sweep-stuck-webhook-events.test.ts__tests__/security/dm-channel-org-precedence.test.ts__tests__/stream/batch.test.ts__tests__/stream/recording-capability.test.ts__tests__/stream/recording-consent.test.ts__tests__/stream/stream-client.test.tsactions/maintenance/drain-sessions.tsactions/stream/chat/user.action.tsapp/api/admin/maintenance/route.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]/recording-consent/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]/components/MeetingSetup.tsxapp/meetings/[id]/components/RecordingConsentNotice.tsxapp/meetings/[id]/hooks/useGetCallById.tsapp/meetings/[id]/page.tsxjobs/meetings/reconcile-orphaned-sessions.tsjobs/stream/expire-event-channels.tslib/meeting.tslib/meetings/access.tslib/moderation/side-effects.tslib/payments/webhooks/handlers.tslib/rate-limit.tslib/stream-cache.tslib/stream-channel-ids.tslib/stream-client.tslib/stream-utils.tslib/stream/appointment-channels.tslib/stream/batch.tslib/stream/call-cid.tslib/stream/health.tslib/stream/recording-consent.tslib/stream/recording-service.tslib/stream/recording-utils.tslib/stream/webhook-dispatch.tslib/stream/webhook-events.tslib/webhooks/event-log.tsmiddleware.tsprisma/schema.prismaproviders/StreamProviderImpl.tsxscripts/cleanup/sweep-stuck-webhook-events.tsscripts/stream/ensure-call-type-grants.tsscripts/stream/ensure-webhook-subscription.tsscripts/stream/stream-sync.ts
teetangh
force-pushed
the
fix/stream-known-open
branch
2 times, most recently
from
August 12, 2026 18:26
e7e8845 to
a2e6c2d
Compare
teetangh
force-pushed
the
fix/stream-known-open
branch
4 times, most recently
from
August 12, 2026 19:33
df6b91a to
2231b93
Compare
This was referenced Aug 12, 2026
teetangh
force-pushed
the
fix/stream-known-open
branch
from
August 12, 2026 20:18
2231b93 to
bdf95f9
Compare
teetangh
added a commit
that referenced
this pull request
Aug 12, 2026
…g calls Three follow-ups on this PR's own code, moved here from downstream because this is the PR that introduces the thing being fixed. The drain freezes group chat channels on the way into OFFLINE. The unfreeze was sitting four PRs later in #1141, which meant merging this one alone left a release window where ending maintenance silently bricked group chat: Stream grants `use-frozen-channel` to NO role by default, so a channel left frozen is unwritable by every user and every admin with no visible cause. A PR that introduces a freeze ships its inverse. `unfreezeChannelsAfterMaintenance` is wired into the maintenance exit, scoped to sessions the drain actually ended (`endedReason: "maintenance"`) inside the recent window so it cannot unfreeze a channel a moderator froze deliberately, and its partial failures are both reported to Sentry and returned to the operator — a silent unfreeze failure is indistinguishable from success, and invisibility is the whole problem with a frozen channel. The `maintenance.draining` custom event is gone rather than bounded. My previous commit put a 2s deadline on it, which was fixing the wrong layer: nothing in the client subscribes to `call.on("custom", …)`, and `end()` fires microseconds later, so no toast could paint even with a listener. Counting Stream's acknowledgement as a person warned is the same fabricated metric this function was already fixed for twenty lines below. Restore it together with a subscriber in MeetingRoom.tsx and a delay before end() — not before. The circuit breaker on the channel freeze was in the wrong position, and my previous commit only half-fixed it. Wrapping `Promise.allSettled` reports failures into `result.errors` but the breaker still sees a resolved promise however many channels failed — it cannot trip during the very outage it exists for. The breaker belongs inside the map, per channel, which is what the unfreeze already did. `lib/stream/recording-service.ts` had zero breaker coverage. Three of its sixteen methods touch Stream (`startRecording`, `stopRecording`, `getCallRecordingsFromStream`); the rest are Prisma-only. This matters twice for `stopRecording`, which the maintenance drain calls in a loop of up to MAX_DRAIN_BATCH sessions — an unbounded call there holds the OFFLINE transition open for the length of the outage it is transitioning for. That was a second unbounded path in the same loop as the one bounded last commit. Also verified and NOT changed: review flagged as Critical that the workflow never invokes the reconciler. It does. The package has no `"type"` field, so tsx runs CommonJS, `require.main === module` is true when the workflow executes the file directly, and the guard exists because `app/api/cleanup/reconcile-sessions` imports the same function. tsc clean, eslint clean, 238 suites / 2698 tests. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
teetangh
force-pushed
the
fix/stream-known-open
branch
from
August 12, 2026 20:33
bdf95f9 to
a748e0d
Compare
teetangh
added a commit
that referenced
this pull request
Aug 12, 2026
…r that never ran (#1137) * fix(stream): make the webhook path durable and schedule the reconciler (#1134) The P1 durability cluster. Every one of these compounds the P0-5 webhook outage: even once the secret is set, nothing was going to survive a transient failure. P1-1 — reconcile-orphaned-sessions has NEVER run. Its docstring claimed "every 30 minutes via cleanup API route"; there are 64 workflows and none invoked it, and nothing called the route. It is the only backstop for a session whose `call.session_ended` never landed, which — with the secret missing — was every session. Added the workflow. Also fixed two bugs in the job itself: it was the one site passing `streamCallId` to Stream without normalising the cid (so a prefixed value always 404'd and was recorded UNVERIFIED), and it had no breaker, so a Stream outage meant 100 sequential 30s timeouts per run. P1-2 — Stream retries at most five times inside a FIFTEEN SECOND total budget, six seconds per attempt, then drops the event forever. The route was doing a DB health probe, an idempotency read, the handler and the completion mark inside that, on a platform where this repo has measured ~30s of event-loop stall on instance boot. It now verifies the signature, acknowledges, and processes in after(). Durability moves to sweep-stuck-webhook-events, which previously filtered `provider: "razorpay"` and would never have touched a Stream row. That required moving the dispatch out of the route — a Next route module cannot export anything but its HTTP handlers, and the sweeper needs to call it. The schemas and switch now live in lib/stream/webhook-dispatch.ts. The three shared bookkeeping helpers moved from app/api/webhooks/utils.ts down to lib/webhooks/event-log.ts (lib/ may not import from app/), re-exported from their old home so no existing caller changes. P1-9 — idempotency now keys on Stream's `X-Webhook-ID`, which is documented as stable across the retries of one delivery. The hand-rolled key collapsed to `stream_<type>_chat_<created_at>` for chat events, so two flags in the same second deduped to one, and participant joined/left omitted the user id, so two people joining in the same second collapsed into a single attendance write. P1-3 — the maintenance drain selected `{ endedAt: null }` and called that "active". That matched 1,663 rows going back months, each of which it would have ended on Stream serially and stamped `endedReason: "maintenance"` — rewriting the history of sessions that finished in February. Now bounded to a 6h window with a 200-row cap. It also never set SlotOfAppointment.completionStatus, so a drained session sat SCHEDULED forever and its earnings never became releasable; and `result.notified` reported a per-participant count for what is actually a platform-wide broadcast taking no recipient list. Both fixed, and participants now get an in-call event telling them why they were disconnected. P1-4 — nothing froze chat during maintenance, so messages kept flowing while the app was offline and unable to sync or moderate them. Group channels for drained appointments are frozen. P1-5 — one `call_cid` helper (lib/stream/call-cid.ts) replaces four reimplementations of the `type:id` split. #473 — the breaker covered chat only; every video/server path called getStreamVideoClient() raw, and /api/health had no Stream check. Both closed. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(ci): stagger the reconciler cron off reconcile-orphaned-confirmations `13,43` is reconcile-orphaned-confirmations' slot (`13-59/30`). Two half-hourly jobs on the same minute start together every single time rather than occasionally, and scripts/ci/check-workflow-hygiene.ts fails the build on exactly that — which is why this PR and every PR stacked above it have been red since it was opened, not because of anything they changed. Of the sixty minutes, only :00, :03, :10, :15, :25 and :55 are unclaimed by an existing sub-daily schedule, and :25/:55 is the only pair thirty minutes apart. `check-workflow-hygiene` now reports ok across 65 workflows with no recurring start collisions. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(ci): harden the reconciler workflow's install steps SonarCloud flags three supply-chain issues on this workflow (githubactions:S6505 ×2, S8543): `npm ci` and `npx` both run lifecycle scripts by default, and the runner holds deploy-scoped secrets — so a compromised transitive dependency's postinstall would run with them. `npx --yes` without a pinned version compounds it. Matched to the posture expire-reschedule-proposals.yml already uses: `npm ci --ignore-scripts`, `npx --no-install --ignore-scripts prisma generate`, and `--ignore-scripts` on the pinned tsx invocation. Nothing here needs lifecycle scripts — the workflow runs `prisma generate` explicitly on the next step, which is exactly why the existing pattern does both. These were failing the `new_security_rating` quality gate at 3, not any test. 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 durability argument non-circular Seventeen review findings routed to this PR by file ownership; ten were legit and they share one root. The ack-first design is right and stays — Stream retries inside a 15-second total budget at 6 seconds per request, which a cold Netlify instance cannot fit a full handler into. What was wrong was the durability ARGUMENT, which was circular: it said durability comes from the WebhookEvent row and the sweeper, while the row itself was written inside `after()`, on the far side of the acknowledgement. Three paths therefore lost a first delivery silently, and in every one Stream had already been told 200 and would never redeliver: the instance freezing before `after()` ran; the DB-health probe returning early before anything was written; and logWebhookEvent itself failing. The sweeper can only re-drive rows that exist, so none was recoverable. `recordStreamEventReceipt` splits out the durable half so the route can call it BEFORE acknowledging. One indexed insert of an already-parsed body fits inside the six-second timeout where the handler does not — which is the whole reason the handler moved to `after()`. A failure there now returns 503 so Stream redelivers, which is correct precisely because nothing was recorded. The in-progress guard was defeated for every row it most needed to protect. The retry path reset `processed` and `error` but never `receivedAt`, and the staleness escape measures `now - receivedAt` — so a retried row was instantly older than the five-minute threshold and any worker would re-claim an event another was mid-way through. Both escapes were also check-then-act: read, decide, write, with two workers able to win simultaneously. Both are conditional writes now, and the affected row count IS the claim. The handled-event list and the dispatch switch were two independent lists that could drift silently. They are bound by a `never` assertion in the default branch now. Written first as `eventType as never`, which compiles unconditionally and verifies nothing — the same zero-branch shape this audit keeps finding. The cast is gone; the guard narrows first, and adding a list entry without a case now fails tsc. Verified by adding a fake entry and watching it fail. Malformed JSON returned 500 rather than 400, because JSON.parse throws SyntaxError and only ZodError was checked — so Stream spent its whole retry budget redelivering a body that could never parse. The fallback event key also dropped `.filter(Boolean)`, which collapsed positions and could align two different events onto one key. In the drain: it stamped a session ended even when call.end() failed or the breaker tripped, so a call could still be live and billing while the row claimed it finished, with nothing to revisit it. `endedAt` is only written on a confirmed end now. The window bounded `endsAt` but not `startsAt`, so a room opened early would be drained before it happened. The courtesy warning sat outside the breaker with no timeout on the video client — 200 serial iterations of an unbounded call during the very outage it announces — and is now deadlined at 2s. And `Promise.allSettled` inside the circuit breaker meant the breaker recorded success however many channels failed to freeze, with not one failure reaching result.errors. Declined: pinning actions to SHAs, since zero of 66 workflows do and that is a repo-wide decision, not a gate on this PR. Added `permissions: contents: read`, matching expire-reschedule-proposals.yml. tsc clean, eslint clean, 238 suites / 2698 tests. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): ship the freeze with its inverse, and bound the recording calls Three follow-ups on this PR's own code, moved here from downstream because this is the PR that introduces the thing being fixed. The drain freezes group chat channels on the way into OFFLINE. The unfreeze was sitting four PRs later in #1141, which meant merging this one alone left a release window where ending maintenance silently bricked group chat: Stream grants `use-frozen-channel` to NO role by default, so a channel left frozen is unwritable by every user and every admin with no visible cause. A PR that introduces a freeze ships its inverse. `unfreezeChannelsAfterMaintenance` is wired into the maintenance exit, scoped to sessions the drain actually ended (`endedReason: "maintenance"`) inside the recent window so it cannot unfreeze a channel a moderator froze deliberately, and its partial failures are both reported to Sentry and returned to the operator — a silent unfreeze failure is indistinguishable from success, and invisibility is the whole problem with a frozen channel. The `maintenance.draining` custom event is gone rather than bounded. My previous commit put a 2s deadline on it, which was fixing the wrong layer: nothing in the client subscribes to `call.on("custom", …)`, and `end()` fires microseconds later, so no toast could paint even with a listener. Counting Stream's acknowledgement as a person warned is the same fabricated metric this function was already fixed for twenty lines below. Restore it together with a subscriber in MeetingRoom.tsx and a delay before end() — not before. The circuit breaker on the channel freeze was in the wrong position, and my previous commit only half-fixed it. Wrapping `Promise.allSettled` reports failures into `result.errors` but the breaker still sees a resolved promise however many channels failed — it cannot trip during the very outage it exists for. The breaker belongs inside the map, per channel, which is what the unfreeze already did. `lib/stream/recording-service.ts` had zero breaker coverage. Three of its sixteen methods touch Stream (`startRecording`, `stopRecording`, `getCallRecordingsFromStream`); the rest are Prisma-only. This matters twice for `stopRecording`, which the maintenance drain calls in a loop of up to MAX_DRAIN_BATCH sessions — an unbounded call there holds the OFFLINE transition open for the length of the outage it is transitioning for. That was a second unbounded path in the same loop as the one bounded last commit. Also verified and NOT changed: review flagged as Critical that the workflow never invokes the reconciler. It does. The package has no `"type"` field, so tsx runs CommonJS, `require.main === module` is true when the workflow executes the file directly, and the guard exists because `app/api/cleanup/reconcile-sessions` imports the same function. tsc clean, eslint clean, 238 suites / 2698 tests. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv * fix(stream): stop an empty error message reading as success Four accuracy and correctness items from review, one of which is a real defect in the state machine this PR just hardened. `markWebhookEventProcessed` wrote `error: error || null`. A handler throwing `new Error("")` produces an empty `processingError`, and `"" || null` collapses to null — which is the SUCCESS shape in the three-state machine. The event had failed and was permanently marked handled, and the sweeper's selector explicitly skips `processed=true, error=null`, so nothing would ever revisit it. `??` now, with an empty message becoming a readable placeholder rather than a silent success. Its docstring also claimed processed=true is set "only on success", which the implementation contradicts and has to: processed=true with a non-null error IS the FAILED state that logWebhookEvent re-drives. The route asserted "at most five times" in two comments. Stream's own documentation contradicts itself here — the webhooks overview gives 3 attempts for 408/429/5xx and 2 for network errors, while their retries announcement says "a maximum of five attempts, whichever comes first". Both agree on six seconds per request inside a fifteen-second total budget, and the budget is what this design turns on, so the comments state that and no longer assert a count. The sweeper's terminal marker was `gave up: payment never arrived`, written for Stream events too now that the sweep covers both providers — sending whoever reads the row looking for a payment that was never involved. It is provider-aware, keeping the `gave up:` prefix that the selector matches on. And `processStreamEvent`'s outer catch still warned the event "may be lost" because no row would exist for the sweeper. That stopped being true one commit ago, when the receipt moved ahead of the acknowledgement. Still paged on — broken completion bookkeeping is worth knowing — but it no longer describes a loss that cannot happen. tsc clean, eslint clean, 238 suites / 2701 tests. 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>
teetangh
force-pushed
the
fix/stream-known-open
branch
2 times, most recently
from
August 12, 2026 21:17
2b001e9 to
2cce246
Compare
teetangh
force-pushed
the
fix/stream-known-open
branch
2 times, most recently
from
August 12, 2026 21:37
d41ba2b to
6a81a0f
Compare
teetangh
force-pushed
the
fix/stream-known-open
branch
4 times, most recently
from
August 13, 2026 04:48
b3c8c54 to
8eff760
Compare
teetangh
force-pushed
the
fix/stream-known-open
branch
2 times, most recently
from
August 13, 2026 05:13
4f9e463 to
f5d6f75
Compare
…ntenance (#1134) The four items left open after two rounds of adversarial validation against the live Stream app. 1. The webhook subscription covered SIX event types while the dispatcher handles TEN. The four missing ones were never delivered, so two features shipped as dead code: MeetingAttendance was never written (detect-consultant-no-shows has been running daily against a permanently empty table, and #471/#472 were never actually unblocked), and every chat moderation flag landed in a queue nothing fed. This is the SECOND independent cause of the zero-attendance figure — the first was the missing webhook secret. Fixing one without the other changes nothing for attendance, which the P0-5 write-up did not make clear. scripts/stream/ensure-webhook-subscription.ts fixes it, dry-run by default. Verified against the live app: it correctly reports the five gaps (participant joined/left, session_started, user.flagged, message.flagged). It unions rather than replaces, so another integration relying on event types we do not handle is not silently unsubscribed. HANDLED_EVENT_TYPES moved to lib/stream/webhook-events.ts — a dependency-free module, because webhook-dispatch transitively imports Prisma, Supabase and `server-only`, none of which load in a bare tsx process. One list, two consumers, so handling and subscribing can never drift again. 2. Frozen channels were never unfrozen. The drain freezes group chat on the way into OFFLINE and the helper's own docstring claimed "unfrozen again by the maintenance exit path" — no such path existed. Worse, Stream grants `use-frozen-channel` to NO role by default, so every channel a drain touched stayed unwritable by every user AND every admin, permanently, with no visible cause. unfreezeChannelsAfterMaintenance() is wired into the DELETE handler, scoped to sessions this drain actually ended so it cannot unfreeze one a moderator froze deliberately. 3. The retry budget is 3 attempts (2 on a network error), not 5. Wrong in two places in the webhook route. 6s per attempt and 15s total were right. The real budget is tighter than documented, which makes acking first more necessary rather than less. 4. sendCallEvent warned nobody. Nothing subscribes to call.on("custom"), and end() fires microseconds later, so no toast could paint even with a listener. Counting Stream's acknowledgement as a person warned is the same fabricated metric this function was fixed for twenty lines below, so the call and the counter are gone rather than left implying a courtesy that does not exist. `notified` now honestly describes broadcast reach and is only set after the broadcast succeeds. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
…r suggests SonarCloud raises typescript:S2871 (CRITICAL, ×2) on the two bare `.sort()` calls here, which is what has been failing the `new_reliability_rating` gate at 4. The rule is right that a comparator should be explicit. Its suggested remedy is not: the message recommends `localeCompare`, which would be a real bug in this file rather than a style change. These are event-type strings — `call.session_started`, `message.flagged`, `call_member`. ICU collation treats `.` and `_` as ignorable punctuation at the primary level; code units do not. So the two orderings genuinely disagree, and this sorted list is compared against the live hook's `event_types` to decide whether an update is needed. A locale-dependent order makes that decision depend on the runtime's ICU build — the same failure mode as the DM channel ids in #1134 P0-3, where a "standardize the conventions" refactor swapped `.sort()` for `.sort(localeCompare)` and silently re-keyed most pairs. A bare `.sort()` on strings already compares by code unit, so behaviour is unchanged. The comparator just states it, and the comment says plainly not to "fix" it to localeCompare, because Sonar will keep suggesting exactly that. Part of #1134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
…hook
`updateAppSettings({ event_hooks })` REPLACES the array. The script submitted
`[theOneHookBeingWidened]` from inside a loop over hooks, so applying it deleted
every other hook on the app — including the SQS and Pusher hooks that the
`hook_type === "webhook"` filter removes before the loop even sees them, and any
second webhook another integration owns.
Writing inside the loop compounded it. Each payload was built from a read taken
before the previous iteration's write landed, so with two hooks to widen only the
last one's widening would have survived.
The comment above it said "Union, never replace", which is true of the event
TYPES and false of the hooks ARRAY. That is why it read as safe.
Now: collect the widenings, then make ONE write carrying every hook the app has,
with only the targeted ones modified.
Latent today — this app has exactly one hook, which is why a dry run looked
correct. That is not a defence. This is the operator script for a shared
production Stream app with no rehearsal environment, run by hand, and the first
time someone adds a second hook it would have silently destroyed it.
Pinned by tests that were verified to FAIL against the previous implementation:
reinstating the per-hook write fails exactly the three cases about preserving
foreign hooks and widening two in one write.
tsc clean, eslint clean, 242 suites / 2734 tests.
Part of #1134
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
teetangh
force-pushed
the
fix/stream-known-open
branch
from
August 13, 2026 13:06
f5d6f75 to
0f815bf
Compare
`getAppSettings()` already returns `AppSettingsAPIResponse`, whose `app` carries `event_hooks?: Array<EventHook>` (stream-chat types.d.ts:101), and `updateAppSettings(options: AppSettings)` accepts `event_hooks` directly (types.d.ts:1842). Both casts predated a check of the installed types and were hiding the SDK's own shape — a future response change would have compiled clean. `EventHook.id` is optional in the SDK because the id is server-generated on create, so the widening candidates are narrowed to hooks that actually have one. The preservation list stays unfiltered: it is what stops the write deleting every other hook. Part of #1134
|
teetangh
added a commit
that referenced
this pull request
Aug 13, 2026
…1149) * fix(stream): close the review tail on files the train already merged (#1134) Eight findings from the #1141 review that live on files now merged to dev. They cannot be fixed in #1141 — its diff owns four files — so they land here as a sibling off dev. No overlap with #1142 or #1143. **The TRIAL DM branch was unreachable.** #1136 added a `TRIAL` case to payment-success channel provisioning, and it has never run. The whole block is guarded by `if (appointmentForChannel && consultantUserId)`, and the consultant was resolved from consultation/subscription/webinar/class only. A trial appointment has none of those — the consultant is a REQUIRED relation on `TrialSession`, a model the query did not include. So the resolution came back undefined for exactly the appointments the branch was written to serve. A trial buyer got video and no way to message their consultant, which is the failure #1136 set out to fix, reintroduced one layer up. Resolved from `TrialSession.consultantProfile`, not `trialSession.subscriptionPlan.consultantProfile` — the latter is the plan author and can differ from whoever is running the trial. **The consent endpoints loaded the meeting twice** and picked their HTTP status by comparing `access.message` to `"Meeting not found"` — the exact coupling the resolver's own docblock warns against, still live in two handlers. `resolveMeetingAccess` already joins all four plan relations for its ownership test, so `recordingEnabled` is one more column on an existing join, not a query; the resolver now hands back what it loaded and the second round trip is gone. `MeetingAccess` becomes a discriminated union while doing it. "Present only when the meeting exists" was a comment on optional fields; it is now a fact the compiler enforces, and narrowing on `hasAccess` gives callers the appointment non-optionally instead of a `!`. Also: - The channel-expiry job reported `success: true` when Stream was not configured. #1134 found a webhook secret silently unset in Netlify; this would have shown that as a green nightly run for as long as it lasted. - `prisma.$disconnect()` moved into a `finally` — a throw skipped it. - `syncUserEventChannels` signals failure by RESOLVING `{success: false}`, so the `.then` persisted "synced" to sessionStorage on failure and suppressed the retry for the rest of the tab's life. - The consent hook re-checks `cancelled` after `res.json()`; that second await could land an old meeting's notice in the state that gates Join. - The Join button sat disabled and unlabelled while the notice fetched. - `stream-sync`'s `requireLock` doc said `default: false` and its warning said "proceeding without distributed lock". #1134 P1-21 made it default true and throw, so both sent a reader looking for a run that never happened. 243 suites / 2,749 tests, tsc and eslint clean. Part of #1134 * refactor(stream): re-export resolveAppointmentPlan directly (#1134) Imported only to be re-exported, which routes the binding through this module for no reason. `export … from` says the same thing in one line. Part of #1134
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The four items left open after two rounds of adversarial validation against the live Stream app.
1. The webhook subscribed to 6 of the 10 events we handle
The live hook carried six event types while
lib/stream/webhook-dispatch.tshandles ten. The four missing ones were never delivered, so two features shipped as dead code:call.session_participant_joined/_left→MeetingAttendancewas never written.detect-consultant-no-showshas been running daily against a permanently empty table, and [RESILIENCE] No-show detection and handling system #471/[RESILIENCE] Session overrun detection and conflict prevention #472 were never actually unblocked.user.flagged/message.flagged→ every report written by the chat UI landed in a queue nothing fed.This is a second, independent cause of the zero-attendance figure. The first was the missing webhook secret (#1136 P0-5). Fixing one without the other changes nothing for attendance — the original write-up implied a single root cause, and that was wrong.
scripts/stream/ensure-webhook-subscription.ts, dry-run by default. Verified against the live app:It unions rather than replaces, so another integration relying on event types we don't handle isn't silently unsubscribed.
call.session_startedis included even though the dispatcher doesn't handle it yet — an unhandled event is a cheap no-op, an unsubscribed one can't be recovered after the fact, and it's what would let us record when a call actually started instead of computing every duration from the scheduled slot time.HANDLED_EVENT_TYPESmoved tolib/stream/webhook-events.ts— dependency-free, becausewebhook-dispatchtransitively imports Prisma, Supabase andserver-only, none of which load in a baretsxprocess. One list, two consumers, so handling and subscribing can't drift again. That drift is exactly what this PR is fixing.2. Frozen channels were never unfrozen — and admins were locked out too
The drain freezes group chat on the way into
OFFLINE, and the helper's own docstring claimed "Unfrozen again by the maintenance exit path." No such path existed.Worse: Stream's docs are explicit that
use-frozen-channelis granted to no role by default. So every channel a drain touched became permanently unwritable by every user and every admin, with no visible cause and no way to recover from the UI.unfreezeChannelsAfterMaintenance()is now wired into theDELETEhandler, scoped to sessions this drain actually ended (endedReason: "maintenance"inside the recent window) so it can't unfreeze one a moderator froze deliberately. UsesupdatePartial—channel.update()is a full replace that would deleteorganizationId,appointmentIdand every other custom field off the channel.3. The retry budget is 3, not 5
Verbatim from Stream: "Response code is 408, 429 or >=500: 3 attempts. Network error: 2 attempts." The 6s-per-attempt and 15s-total figures were right; the attempt count was wrong in two places in the webhook route. The real budget is tighter than documented, which makes the ack-first design more necessary, not less.
4.
sendCallEventwarned nobodyThe API call was correct and does fire a
customWS event — but nothing in this repo subscribes tocall.on("custom", …), andcall.end()fires microseconds later, so no toast could paint even with a listener.Counting Stream's acknowledgement as a person warned is the same fabricated metric that function was fixed for twenty lines below. Both the call and the counter are gone rather than left implying a courtesy the product doesn't provide.
notifiednow honestly describes broadcast reach and is only set after the broadcast succeeds.Restoring an in-call warning needs a
call.on("custom")subscriber plus a delay beforeend()— worth doing, but as a change that actually works rather than one that looks like it does.Verification
tsc --noEmitclean,eslintclean at zero warnings.Operator action
Safe to run before or after the other merges — it only widens a subscription. Without it, attendance and chat moderation stay dead no matter what else lands.
Part of #1134
🤖 Generated with Claude Code
https://claude.ai/code/session_019pbBn6yWAr2DXjfACyocUv
Summary by CodeRabbit
New Features
Improvements
Bug Fixes