Fix GitHub App setup race and tidy integrations cards - #538
Merged
Merged
Conversation
GitHub sends the installation webhook at the same moment it redirects the user to the setup URL. The webhook could save the installation while the setup request was still checking it with GitHub, so the setup's create() hit the unique key on installation_id. Setup now uses createOrFirst() and checks who owns the installation afterwards. The cards on the integrations page now share the Flux card radius and none has a shadow. The App Installations card is a Flux card like the GitHub Account card above it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Fixes Nightwatch issue 97, a 500 on
/auth/github/setupwith a duplicate key ongithub_installations.installation_id.GitHub sends the
installation.createdwebhook at the same moment it redirects the user to the setup URL. The setup request looked up the installation, didn't find it, then asked GitHub's/user/installationswhether it belonged to the user. The webhook saved the row while that call was in flight, so the setup'screate()hit the unique key.Setup now uses
GitHubInstallation::createOrFirst(). If the webhook got there first, setup picks up that row and syncs it as usual. The "already linked to another NativePHP account" check has moved below the lookup, so it also catches a row that appears mid-request under a different user.createOrFirst()is called on the model rather than through$user->githubInstallations()because the relation version only looks for the current user's rows and would rethrow in that case. The webhook side already usedupdateOrCreate(), which copes with losing the race.Two new tests in
GitHubAppSetupTestfake GitHub so the row appears while the request is checking. Both failed with the production error before the fix.This also tidies the cards on the integrations page, which had mixed corner radii and one had a shadow. The App Installations card is now a Flux card like the GitHub Account card above it, so it loses the shadow and its oversized heading. The claude-code, mobile and Discord cards and the GitHub migration banner go from
rounded-lgtorounded-xlto match Flux cards and callouts. Colours are unchanged apart from the Installations card's dark mode background, which was navy and now matches the other plain cards. The migration banner also appears on the plugin index and create pages, so those pick up the new radius too.🤖 Generated with Claude Code