feat(functions)!: rename set_auth_token to auth_token - #90
Draft
spydon wants to merge 1 commit into
Draft
Conversation
The capability was specified as a mutator: "Update the authorization token sent with all subsequent function invocations." That prescribes a shape rather than a behaviour, and the shape it prescribes is the one two SDKs have now removed. supabase-swift replaced FunctionsClient.setAuth(token:) with an access token closure resolved per request, and supabase-flutter is removing FunctionsClient.setAccessToken() for the same reason: a pinned token outlives the session that was current when it was set, shadowing that session across refreshes and sign-outs with nothing to clear it. supabase-csharp never had a setter at all and sits at partially_implemented purely because of the wording. Renamed to functions.invocation.auth_token, described in terms of what a caller can observe: which token an invocation sends, and which source wins. A stateful setter still satisfies it; it is no longer the only thing that does. Realtime's set_auth_token keeps its name. It genuinely is a mutator, since it holds a live socket and pushes a new token over it rather than attaching one per request, and both PRs above deliberately kept it. cross_client_token_sync had the same problem in its description, which specified propagation on auth state change events as the mechanism. supabase-py and supabase-csharp already carry notes explaining that they do it another way, and per-request resolution now makes a third. Reworded around the guarantee, leaving the mechanism open. Refs: supabase/supabase-swift#1233, supabase/supabase-flutter#1739
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
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.
Summary
Renames
functions.invocation.set_auth_tokentofunctions.invocation.auth_tokenand respecifies it in terms of observable behaviour instead of a mutator method.The old entry read: "Update the authorization token sent with all subsequent function invocations." That describes an implementation shape, not a capability, and it is the shape two SDKs have now deliberately removed.
FunctionsClient.setAuth(token:)with anaccessTokenclosure resolved fresh on every request, which letFunctionsClientdrop its lock and become astruct.FunctionsClient.setAccessToken()along with the Postgrest and Storage equivalents, sinceAuthHttpClientalready resolves the session token per request.Both did it for the same reason: despite the name, a setter like this pins a token rather than keeping one in sync. The pinned token outlives the session that was current when it was set, shadowing it across refreshes and sign-outs, and nothing ever clears it.
supabase-csharp is the third data point. It never had a stateful setter, supplies the bearer token per invocation or through a
GetHeadershook, and sits atpartially_implementedwith the note "No stateful SetAuth" — marked down purely by the wording, not by any behaviour a user could miss.The new entry asks what a caller can observe: which token an invocation sends, and which source wins when more than one applies. A stateful setter still satisfies it. It is no longer the only thing that does.
What changed
capabilities/functions.yaml— the rename, plus a description covering the three ways a token reaches an invocation: fixed at construction, resolved per request from an access token provider, or overridden on a single call.specs/functions/invocation/auth_token.md(new) — the resolution order as a numbered precedence, and two notes worth calling out because they are the failure modes actually found in review on the PRs above:Authorizationbreaks this while still looking like it implements the capability. That was a real bug in supabase-swift, found in review on #1233 and fixed there; supabase-flutter avoids it viaputIfAbsentand #1739 adds an end-to-end test pinning that down.capabilities/client.yaml— no ID change, description only.cross_client_token_synchad the same defect: it specified "listens for auth state change events and automatically propagates the current JWT to all sub-clients" as the mechanism. supabase-py (lazily recreates sub-clients on next access) and supabase-csharp (sharedGetAuthHeaderscallback) already carry notes explaining that they do it differently, and per-request resolution in Swift and Flutter is now a third way. Reworded around the guarantee, with the mechanism explicitly left open.What is deliberately not changed
realtime.client.set_auth_tokenkeeps its name. Realtime is the one case where a setter is the right primitive: it holds a live socket and has to push a new token over it rather than attach one per request, which is whySupabaseClientgenuinely calls it on auth state changes. Both PRs above kept it for exactly that reason. The naming asymmetry that leaves is a real signal rather than an inconsistency, and the new spec says so.Why draft — this breaks five repos on merge
There is no alias or deprecation mechanism for feature IDs, and
validateCompliancetreats an unrecognised ID as a hard error, not a warning. Every SDK compliance file still naming the old ID fails validation the moment this lands. Verified locally against each file fetched from its default branch:supabase-go is unaffected; its compliance file is sparse and never declared the ID.
The reverse direction is harmless: a missing ID is informational only, treated as
not_implemented, so nothing breaks in the window between this merging and a repo updating. Only the stale key is fatal. Follow-up PRs, one per repo, each a key rename:sdk-compliance.yaml:977implemented, symbolFunctionsClient.setAuthsdk-compliance.yaml:1968implemented, symbolFunctionsClient.setAccessToken— superseded by #1739, which already rewrites this entrysdk-compliance.yaml:462implemented, symbols{Async,Sync}FunctionsClient.set_authsdk-compliance.yaml:592implemented(bare)sdk-compliance.yaml:385partially_implemented— should becomeimplementedunder the new wording; the note explains an absence that is no longer a gapWorth sequencing so the compliance PRs are open and ready before this merges, rather than landing this and leaving five repos red. Happy to open them.
Test plan
npm run validate— capability matrix validnpm test— 195 passing, 14 filesnpm run typecheck— cleannpm run validate-complianceagainst all six SDK compliance files, to establish the breakage above rather than discover it after merge