diff --git a/docs/decisions/2026-08-13-mid-session-recording-decline.md b/docs/decisions/2026-08-13-mid-session-recording-decline.md new file mode 100644 index 000000000..0e931d8ff --- /dev/null +++ b/docs/decisions/2026-08-13-mid-session-recording-decline.md @@ -0,0 +1,118 @@ +# ADR: A mid-session recording decline stops the recording and discards it + +- **Status**: Accepted +- **Date**: 2026-08-13 +- **Author**: teetangh +- **Part of**: #1134, #1146 + +## Context + +#1139 shipped pre-join recording consent. Before it, a consultee's first sign +that a session was being recorded was a small `REC hh:mm` pill that appeared +once recording had already started: no notice, no way to refuse, and no record +that anyone had been told, on a product whose one-to-one sessions are career and +health conversations. + +That work deliberately covered one moment only. `lib/stream/recording-consent.ts` +carries an explicit scope note saying enforcement is start-time only, and the +`DECLINED` check lives inside the atomic claim in +`POST /api/stream/recordings/start`, so a decline arriving between the read and +the write loses the race rather than being ignored. + +What it does not answer is what happens when someone declines **after** recording +has begun. At that moment a recording of them already exists, and every possible +answer costs something real: + +- Discarding it destroys work the consultant may be relying on. +- Keeping it retains a recording of a person who has said they do not want one. +- Refusing the decline tells someone in a health or career conversation that + their withdrawal of consent does not count. + +This is a question about what the platform promises, not about how to implement +it, which is why it sat open in #1146 rather than being decided in code. + +## Decision + +**A mid-session decline stops the recording immediately and discards what has +already been captured.** + +The reasoning is that withdrawing consent means the recording of that person +should not exist. Keeping the portion captured before the decline retains +precisely the artefact they objected to, and the fact that it is shorter than +the full session does not change what it is. + +It is also the only answer consistent with what has already shipped. The +one-to-one regime is `OPT_OUT`, and the notice tells people in as many words +that declining is real and costs them nothing: they still join, and the +consultant's recording endpoint refuses. A mid-session decline that quietly kept +a partial recording would make that sentence untrue after the fact. + +Group sessions are unaffected. Their regime is `ACKNOWLEDGE`, where the +recording is part of what attendees bought and was disclosed at purchase, and +the only action available is to understand it. There is no decline to honour. + +## Consequences + +### Positive + +- The consent promise means the same thing before and during a session, so the + pre-join copy stays honest without qualification. +- There is one rule to explain rather than a rule and an exception. +- No retained artefact whose lawful basis depends on reconstructing what someone + had agreed to at a particular minute. + +### Negative + +- A consultant can lose an entire session's recording to a decline made in its + final minutes, with no partial retained. This is a real cost to the person who + did nothing wrong, and the pre-join copy must say plainly that a decline can + arrive at any point, rather than implying the decision is settled at join. +- Deletion has to be genuine, which means the discard path must reach Stream's + stored recording and any transfer already in flight — not merely mark a row. +- Implementation is more than a status flag: it needs an authenticated decline + during the call, a server-side stop, and a delete that is safe to retry. + +### Neutral + +- Nothing about the currently shipped behaviour is wrong today. Enforcement is + start-time only and the source says so explicitly, so this ADR records a + decision to be built, not a defect to be repaired. + +## Alternatives considered + +### Stop and keep what was already captured + +Rejected. It is defensible in the narrow sense that consent was live while that +portion was recorded, but it leaves the platform holding a recording of someone +who has explicitly said they do not want one, and it makes the answer to "what +do you have of me?" depend on the second at which they clicked. On career and +health conversations that is the wrong side to err on. + +### Refuse the decline once recording has started + +Rejected. It is the cheapest to build and the easiest to state contractually, +and it is the option most at odds with what the product already tells people. +The notice presents declining as real and free; making it available only until +the moment it matters would be a dark pattern in a consent flow, which is the +exact thing the #1139 design notes set out to avoid when they gave Decline and +Allow equal visual weight. + +### Ask the consultant to approve the deletion + +Rejected. It makes one participant's consent contingent on another's agreement, +which is not consent. + +## Follow-ups + +Tracked in #1146. The work is a decline path that is authenticated during the +call, a server-side stop, and a delete that reaches Stream's stored asset and +any in-flight transfer, together with pre-join copy that states a decline can +arrive at any time. + +## References + +- #1134 — the Stream audit that surfaced the consent gap. +- #1139 — pre-join consent, and the `OPT_OUT` / `ACKNOWLEDGE` split. +- #1146 — remaining items, where the implementation is tracked. +- `lib/stream/recording-consent.ts` — the scope note recording that enforcement + is start-time only. diff --git a/scripts/stream/ensure-webhook-subscription.ts b/scripts/stream/ensure-webhook-subscription.ts index ad291372e..d9f30b3f9 100644 --- a/scripts/stream/ensure-webhook-subscription.ts +++ b/scripts/stream/ensure-webhook-subscription.ts @@ -69,6 +69,43 @@ const DESIRED_EVENT_TYPES = Array.from( */ type IdentifiedHook = EventHook & { id: string }; +/** + * Event types are partitioned by PRODUCT, and a hook may only carry events from + * its own. + * + * This is not a detail. The live app has exactly one hook, scoped to `video`, + * and `updateAppSettings` refuses the whole write — atomically, so nothing + * lands — if the payload gives it a chat event: + * + * invalid event types for hook 44a1d716-…: event types + * [message.flagged user.flagged] do not belong to product 'video' + * + * The first version of this script had no concept of `product`. It reported all + * five unsubscribed events as simply "missing", which read as one write away + * from fixed, when two of them could never live on that hook at all. Chat + * moderation would have stayed dead with the script reporting success. + */ +const CHAT_EVENT_PREFIXES = ["user.", "message.", "channel.", "member."]; + +function productFor(eventType: string): "chat" | "video" { + return CHAT_EVENT_PREFIXES.some((p) => eventType.startsWith(p)) + ? "chat" + : "video"; +} + +/** + * Whether a hook may carry an event type. + * + * A hook with no `product` is treated as unconstrained: the field is optional in + * the SDK, and refusing to widen a hook we cannot classify would be worse than + * letting Stream reject it with a precise message. + */ +function hookAccepts(hook: EventHook, eventType: string): boolean { + const product = (hook as { product?: string }).product; + if (!product || product === "all") return true; + return product === productFor(eventType); +} + export async function ensureWebhookSubscription(apply: boolean): Promise { if (!isStreamConfigured()) { console.error( @@ -100,21 +137,29 @@ export async function ensureWebhookSubscription(apply: boolean): Promise let changed = 0; /** hook id -> its widened event_types. Applied in ONE write after the loop. */ const widened = new Map(); + /** Events no hook on this app is allowed to carry. */ + const unplaceable = new Set(DESIRED_EVENT_TYPES); for (const hook of hooks) { const current = new Set(hook.event_types ?? []); // A hook subscribed to "*" already receives everything. const receivesAll = current.has("*"); - const missing = DESIRED_EVENT_TYPES.filter( - (t) => !receivesAll && !current.has(t), - ); - console.log(`\nhook ${hook.id} enabled=${hook.enabled}`); + // Only events this hook's product permits. Offering it anything else makes + // Stream refuse the ENTIRE update, so one impossible event silently costs + // every possible one in the same write. + const eligible = DESIRED_EVENT_TYPES.filter((t) => hookAccepts(hook, t)); + for (const t of eligible) unplaceable.delete(t); + + const missing = eligible.filter((t) => !receivesAll && !current.has(t)); + const product = (hook as { product?: string }).product ?? "unscoped"; + + console.log(`\nhook ${hook.id} enabled=${hook.enabled} product=${product}`); console.log(` url: ${hook.webhook_url}`); console.log(` subscribed: ${current.size}${receivesAll ? " (wildcard)" : ""}`); if (missing.length === 0) { - console.log(" ✅ already covers every handled event"); + console.log(` ✅ already covers every handled ${product} event`); continue; } @@ -132,6 +177,30 @@ export async function ensureWebhookSubscription(apply: boolean): Promise console.log(` → will widen to ${next.length} event types`); } + // Events with nowhere to go. This is a configuration gap the script cannot + // close: creating a hook decides a public URL and starts real deliveries, so + // it belongs to a human, in the dashboard, the same way the "no webhook at + // all" case above does. + if (unplaceable.size > 0) { + const byProduct = new Map(); + for (const t of unplaceable) { + const p = productFor(t); + byProduct.set(p, [...(byProduct.get(p) ?? []), t]); + } + console.error( + `\n⚠️ ${unplaceable.size} handled event(s) have NO hook that may carry them.`, + ); + for (const [product, types] of byProduct) { + console.error(`\n product '${product}' — no hook on this app is scoped to it:`); + for (const t of [...types].sort(byCodeUnit)) console.error(` · ${t}`); + console.error( + ` Create a '${product}' webhook in the Stream dashboard pointing at\n` + + ` /api/stream/webhooks, then re-run. Until then these events are\n` + + ` never delivered and the features behind them stay dead.`, + ); + } + } + // One write, carrying every hook the app has. Two things went wrong with the // per-hook write this replaces. It submitted `[oneHook]`, which replaces the // entire `event_hooks` array — so a second webhook, or an SQS or Pusher hook,