docs(supabase-otp-hook): scope the dead-session 503 to the option that has it - #70
Merged
Merged
Conversation
…t has it
The README stated, unconditionally, that a dead session returns 503
synchronously via the host-side preflight. That holds for Option A only.
The preflight probes the *instance's* sessionScope. Option B leaves that blank
and puts the sending session in `fallbackSessionId`, which is plugin config the
host never sees — so `evaluatePreflight` finds no single session to probe and
skips the check. The route is a sync-response route, so its ack is rendered and
returned before the handler runs; by the time the plugin resolves the fallback
and the send fails, the 200 is already committed.
The consequence on an auth path is the bad kind: Supabase reads 200 {"ok":true}
as delivered and does not retry, the dedup row suppresses a repeat, and the only
trace is the plugin's own `sendText failed (background)` log line.
No code changes. The ack-before-dispatch ordering is deliberate — the comment in
the host explains that blocking it would hold the response for the dispatch
timeout — and the plugin's fire-and-forget send is deliberate too, because
awaiting a slow send risks a 504, a provider retry, and a duplicate OTP. Trading
silent loss for duplicate OTPs on an auth path is not an improvement.
What was wrong was the promise. Both the provisioning list and the Security
section now say which option carries the guarantee, and Option B says plainly
what it gives up.
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 claim that was wrong
The README stated, without qualification:
That holds for Option A only.
Why Option B cannot have it
The host preflight probes the instance's
sessionScope. Option B leaves that blank and puts the sending session infallbackSessionId— plugin config the host never sees.evaluatePreflighttherefore finds no single session to probe and skips thesession-alivecheck, exactly as its own comment says ("wildcard: no single session to probe").The route is a sync-response route, so the ack is rendered and returned before dispatch. By the time the plugin resolves the fallback and the send fails, the
200is already committed. There is no point at which the plugin could turn it into a503.Why that matters here specifically
On an auth path the failure mode is the bad kind. Supabase reads
200 {"ok":true}as delivered and does not retry; the dedup row suppresses a repeat; no DLQ row and no message row are written. The only trace is the plugin's ownsendText failed (background)log line.It is conditional — it needs the Option-B choice and a fallback session with no live engine at request time — and Option A is what the README calls "Simplest". But an operator who picked Option B was told they had a guarantee they did not have.
No code changes, deliberately
Both behaviours in the chain are reasoned decisions with comments to match:
Trading silent loss for duplicate OTPs on an auth path is not an improvement, and neither decision is this change's to overturn. What was wrong was the promise, not the code.
What changed
The provisioning list now says what Option B gives up, and the Security section scopes the
503to Option A. A changelog entry records it.