feat(billing): ask for a GitHub reconnect when program membership cannot be verified - #3185
Open
lohanidamodar wants to merge 5 commits into
Open
feat(billing): ask for a GitHub reconnect when program membership cannot be verified#3185lohanidamodar wants to merge 5 commits into
lohanidamodar wants to merge 5 commits into
Conversation
…erified A GitHub refresh token dies at six months, and revoking the OAuth app kills it outright. Either way Appwrite can no longer confirm Education Program membership, and until now the console said nothing — the check simply failed and the organization drifted on a plan nobody could verify. checkForUsageLimit gains two branches. While the organization is still active and programMembershipUnverifiedSince is set, a warning banner asks for a reconnect without touching readOnly, so the budget and limit paths below it run unchanged. Once the organization is restricted the banner becomes an error, and program_membership_invalid gets its own copy: GitHub has confirmed the account is no longer a student, so reconnecting cannot help and the action points at plans instead. Reconnecting needs no backend work — the OAuth callback binds to whoever is logged in and overwrites the stored tokens — so both entry points reuse one helper, keeping the scopes that mint the refresh token from drifting apart. Account settings gains the same action per GitHub identity, which previously offered only delete.
Console (appwrite/console)Project ID: Tip Schedule functions to run as often as every minute with cron expressions |
Contributor
Greptile SummaryThe PR adds GitHub reconnection prompts for education-program verification failures and a reconnect action for GitHub identities. The latest changes also reconcile program-membership alerts and read-only state when membership fields change without changing the selected organization ID.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the current implementation clears stale membership alerts and recomputes read-only state when warning, restricted, or reverified membership state changes under the same organization ID. Important Files Changed
Reviews (5): Last reviewed commit: "fix(billing): re-run the usage check whe..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
headerAlert.add() no-ops on an id it already holds, so adding with show:true was a one-way door: once an organization reverified, the entry stayed visible in the priority store while its component rendered nothing, and because it outranks the alerts below it, they were blanked rather than shown. Both ids now carry the current condition on every call.
Comment on lines
+493
to
+502
| importance: 11 | ||
| }); | ||
| headerAlert.updateShow('programMembershipRestricted', programMembershipRestricted); | ||
|
|
||
| headerAlert.add({ | ||
| id: 'programMembershipWarning', | ||
| component: ProgramMembershipAlert, | ||
| show: programMembershipWarning, | ||
| importance: 9 | ||
| }); |
Contributor
There was a problem hiding this comment.
When the selected organization is reverified or transitions between warning and restricted states without changing its ID, the layout skips checkForUsageLimit, so these updateShow calls never receive the new state. The obsolete entry remains active in the priority store, suppressing lower-priority banners or continuing to show the warning instead of the access-restricted banner.
Knowledge Base Used: Console application shell
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/stores/billing.ts
Line: 493-502
Comment:
**Alert visibility remains stale**
When the selected organization is reverified or transitions between warning and restricted states without changing its ID, the layout skips `checkForUsageLimit`, so these `updateShow` calls never receive the new state. The obsolete entry remains active in the priority store, suppressing lower-priority banners or continuing to show the warning instead of the access-restricted banner.
**Knowledge Base Used:** [Console application shell](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/appwrite/console/-/docs/console-application-shell.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
… gate checkForUsageLimits returns early when the organization id is unchanged, so an organization reverified or restricted mid-session never re-ran the checks — and the banner it already had stayed put, outranking the alerts below it while rendering nothing. The sync is now its own exported function, called from the layout on every organization change rather than only on a new id. checkForUsageLimit still calls it, so the readOnly and delete-member paths are unchanged, and activeHeaderAlert already recomputes from the store.
The unguarded sync updated the banner but threw away what it returned, while readOnly stayed behind the organization-id gate. An organization restricted mid-session therefore showed "access restricted" with its member, project and resource controls still enabled — the banner and the write gates disagreeing, which is worse than both being stale together. It only ever raises readOnly, never clears it: the flag may already be true for an unpaid invoice or a budget, and this check knows nothing about those.
Deciding readOnly from the membership sync alone could only ever be half right. Raising it left a reverified organization locked; clearing it would have unlocked one held for an unpaid invoice or a budget, which this check knows nothing about. checkForUsageLimit already owns that decision for every reason at once. The layout gates it on the organization id, so it is re-run when status, remarks or the unverified stamp change under a stable id, and left alone otherwise. First sight of an organization still goes through the id-gated path, which runs the rest of the billing checks with it.
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.
What does this PR do?
Appwrite Cloud gives education organizations a free plan through the GitHub Student Developer Program and re-validates that membership against GitHub. Validation uses the OAuth refresh token on the user's GitHub identity — and GitHub refresh tokens expire after six months, while revoking the OAuth app invalidates them immediately.
When that happens the backend cannot verify membership at all. Until now the console said nothing: the check simply failed and the organization drifted on a plan nobody could confirm. Cloud #5459 makes the backend warn the owners three times and then restrict the organization; this is the console side of that.
Re-authorizing needs no backend change — the OAuth callback binds to whoever is currently logged in regardless of how they signed in, and overwrites the stored tokens on the existing identity. So putting the user through the GitHub OAuth flow is the entire fix.
Changes
src/lib/stores/billing.ts— constants for the two newremarkscodes next tobillingLimitOutstandingInvoice, and two branches incheckForUsageLimit, inserted after the outstanding-invoice branch:status === readonlywith either new code: header alert,readOnly.set(true),return. Mirrors the outstanding-invoice branch.activewithprogramMembershipUnverifiedSinceset: header alert only. Noreturn, andreadOnlyuntouched, so the budget/limit/notification paths below run unchanged. This is the window that matters — the org is warned while access is still intact.Two alert ids rather than one, because
headerAlert.addis a no-op when the id already exists; a single id would pin the importance to whichever state was seen first. Restricted uses 11 (matchingTeamReadonlyAlert); the warning uses 9, belowbudgetLimitat 10, so a genuinely blocked org still leads with the blocking banner.organization-[organization]/programMembershipAlert.svelte(new) — modelled onteamReadonlyAlert.svelte, sameHeaderAlert/Buttonshape andhideBillingHeaderRoutesguard, written in runes per AGENTS.md. Three mutually exclusive states, and the copy differs meaningfully between them:program_membership_invalidmeans GitHub has confirmed the account is no longer a student, so reconnecting cannot help and the button points at plans instead.src/lib/helpers/github.ts—reconnectGithubIdentity(), thecreateOAuth2Sessioncall lifted from(guest)/education/+page.sveltewith the sameread:user/user:emailscopes, returning the user to where they were. Shared rather than duplicated because the scopes are what mint the refresh token, and the two entry points must not drift.account/identities.svelte— a Reconnect action per GitHub identity. That table previously offered only delete, so there was no deliberate way for a user to refresh a stale token.trackEventfires before the call, since the OAuth flow redirects away and never returns.Copy
Warning (active) —
type="warning", "Reconnect GitHub to keep your education plan":Restricted, unverified —
type="error", "Access restricted":Restricted, invalid —
type="error", "Access restricted":The warning deliberately names the mechanical cause and avoids implying lost eligibility — a token dies at six months through no action by the member.
Test Plan
None of the 92 check warnings or the lint warnings are in the changed files — both counts are the pre-existing baseline. The one failing unit suite (
oauth2-cimd.test.ts,TypeError: Invalid URLfrom an absent.env) was confirmed pre-existing by stashing the change and re-running: byte-identical failure, same 268 passing.No tests added.
vite.config.tsconfigures aclientvitest project forsrc/**/*.svelte.{test,spec}.ts, but no such file exists in the repo — all 15 suites are pure-logic tests on helpers and stores, andbilling.tshas no existing test file. There is no component-test pattern here to follow.Manual QA once cloud#5459 is deployed:
task-billing-validate-github-student --dryRun=false→ org stays active, warning banner appears, Reconnect GitHub starts the OAuth flow and returns to the same page.status=readonly+remarks=program_membership_unverified→ error banner, Reconnect offered.remarks=program_membership_invalid→ error banner, no GitHub action, View plans instead.Known gap
programMembershipUnverifiedSinceis not yet in the pinned@appwrite.io/consoleSDK model, so it is read through one documented structural cast inbilling.ts, following the idiom already used foraccessedAtandlabelsinauth/user-[user]/. When cloud#5459 merges and the SDK pin is bumped, delete the cast from that one function body — nothing else changes.Out of scope, worth flagging
$readOnlyis cause-agnostic but the copy attached to it is not.organization-[organization]/members/+page.svelte:108-110andheader.svelte:162-169gate the Invite button on$readOnlyand then render "Upgrade to add more members" / "You've reached the members limit for the {plan} plan";+page.svelte:63-70gates project creation the same way. A student org restricted forprogram_membership_unverifiedwill see that plan-limit wording, which is misleading. Fixing it properly means threading a restriction reason through those components.