fix(oauth): degrade instead of dying when a scope predates its ceiling edit - #1060
Open
candidate2-vapi wants to merge 1 commit into
Open
fix(oauth): degrade instead of dying when a scope predates its ceiling edit#1060candidate2-vapi wants to merge 1 commit into
candidate2-vapi wants to merge 1 commit into
Conversation
…g edit
Widening the wizard OAuth app's scope ceiling (`OAuthApplication.scopes`) is
a manual Django-admin edit on both prod regions, so it inevitably lands
separately from the code that starts requesting the scope. In that window
PostHog rejects the authorize request with `invalid_scope` — and it rejects
the WHOLE request, before the consent screen renders. The result is that one
not-yet-live scope locks every user of that program out of login entirely,
rather than degrading only the step that needed it. The existing error copy
says as much ("Re-running the wizard will not help"), which is accurate and
also a dead end.
Declare those scopes in `PENDING_CEILING_SCOPES` and retry the flow once
without them, so the run continues with a reduced grant and only the
dependent step degrades. Bounded to a single extra attempt: the retry
requests nothing pending, so the branch cannot match twice. The dropped
scopes are named in a visible warning and captured for analytics, so this
stays a loud degradation rather than a silent one.
The list ships empty — every scope requested today is live. It is the
checklist entry that pairs with the admin edit: add a scope when you start
requesting it, remove it once the ceiling is confirmed in both regions.
Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
This is me! ~@Twixes |
Twixes
self-requested a review
August 5, 2026 03:21
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.
Problem
Widening the wizard OAuth app's scope ceiling (
OAuthApplication.scopes) is a manual Django-admin edit on both prod regions, so it lands separately from the code that starts requesting the scope.src/lib/constants.tsalready documents the consequence:So during that window, one not-yet-live scope doesn't degrade the step that needs it — it locks every user of that program out of login entirely. The current copy in
oauth-errors.tsdescribes this accurately and offers no way forward: "Re-running the wizard will not help until the ceiling is updated."This is not hypothetical. I hit it running
wizard self-drivingoff #1055, which requests the net-newreplay_scanner:read/replay_scanner:write. Login tous.posthog.comsucceeded, then the callback came backhttp://localhost:8239/callback?error=invalid_scopeand the run was dead. The only way to proceed was to locally comment the two scopes out ofSELF_DRIVING_SCOPE_ADDITIONS— which is exactly what this PR does properly, and automatically.The failure mode is generic: it recurs for every future net-new scope, and the blast radius is the whole program rather than one step. #1055's own sequencing notes ("make the prod OAuth-ceiling edit, then merge") are a process workaround for a robustness gap in the client.
Changes
PENDING_CEILING_SCOPESinprogram-scopes.ts— declares scopes that may not be live in the ceiling yet. Ships empty, since everything requested today is live; it's the checklist entry that pairs with the admin edit.performOAuthFlow— oninvalid_scope, if the request contained pending scopes, drop them and re-enter the existingdo/whilewith a reduced set. Bounded to a single extra attempt: the retry requests nothing pending, so the branch cannot match twice.log.warnand captured asoauth pending ceiling scopes dropped, so this never silently hides a real misconfiguration.config.scopesbecomes a localscopesso the retry, the failure message, and the analytics payload all report what was actually requested on the final attempt.The design tradeoff, stated plainly
This trades "fail loudly, fix the ceiling" for "warn loudly, finish the run with less". I think that's right when the alternative is a total lockout on a manual-edit race, and the empty default means nothing changes until someone opts a scope in. If you'd rather the client stay strict, the alternative worth considering is a CI check asserting every requested scope appears in the README ceiling list — happy to switch. Two notes for reviewers:
Suggested follow-up for #1055
That PR should add its two scopes to
PENDING_CEILING_SCOPESin the same commit that adds them toSELF_DRIVING_SCOPE_ADDITIONS, then drop them once the ceiling edit is confirmed in both regions. That makes the ordering constraint in its "Sequencing" section non-fatal if the halves land out of order.Test plan
pnpm exec vitest run— 1723 pass / 121 files, no failures.src/utils/__tests__/oauth.test.ts: pending scopes are dropped with order preserved; an empty pending list is a no-op; a pending scope that wasn't requested does not shrink the set (so it can't arm the retry and loop); and no baseWIZARD_OAUTH_SCOPESentry may be listed pending — degrading one of those would hand back a token that authenticates but can't run the wizard.pnpm exec prettier --checkandpnpm exec eslintclean on all three changed files.pnpm typecheckreports 25 errors before and after this change — all pre-existing onmain(ReadableStream.destroy,Set.slice, etc., seemingly a Node/TS lib mismatch under Node 26). None in the changed files.Not verified end-to-end against production, and I want to be upfront about it: exercising the retry needs a scope that is genuinely outside the live ceiling, and
PENDING_CEILING_SCOPESships empty. The scope-drop logic and the loop-arming conditions are unit-tested, and the reduced-set path is the same code that runs today for a normal first attempt, but the realinvalid_scope→ retry → consent round trip hasn't been observed. The clean way to confirm is to land #1055 with its two scopes listed pending, before the ceiling edit.LLM context
Investigated and authored with Cursor (Opus 5). The
invalid_scopefailure was reproduced by hand while runningwizard self-driving --local-mcpfrom #1055 againstus.posthog.com; the fix, tests, and verification above were produced in that session.Made with Cursor