From abd444192c1722eab39a14326b6caea1fc06a6fa Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Tue, 4 Aug 2026 18:07:04 +0530 Subject: [PATCH 1/2] fix(vcs): surface invalid installation tokens instead of empty results A VCS installation whose OAuth token can no longer be refreshed made every repository, branch and contents call fail with general_provider_failure. None of the Git surfaces read that error, so the console presented the failure as a successful empty result: repositories.svelte rendered its empty state, the branch selectors reported "No branches available", selectRootModal spun forever, and both updateRepository cards fell back to their "no repository connected" state for a repository that is in fact still connected. Classify the failure and render what actually happened, with the action that fixes it. Three kinds, because the remediation differs: - reconnect: the token is dead, only re-authorizing helps - locked: a concurrent refresh holds the lock, retrying works - provider: the provider itself failed, retry first general_provider_failure covers both a dead token and a provider outage, so the message is the only discriminator the API gives us. If that copy is ever reworded this degrades to the provider branch, which still shows a real error and still offers a reconnect. The repository cards gain an explicit fourth state rather than overloading the existing null sentinel, which already means "no repository connected" and is precisely the state being confused today. createGitDeploymentModal and createGit are migrated to runes, per AGENTS.md, since they had to be touched. --- src/lib/components/git/branchSelector.svelte | 224 ++++++++++++++---- .../components/git/connectRepoModal.svelte | 24 +- src/lib/components/git/index.ts | 1 + .../components/git/installationError.svelte | 141 +++++++++++ .../git/productionBranchFieldset.svelte | 109 +++++++-- src/lib/components/git/repositories.svelte | 120 ++++++++-- src/lib/components/git/selectRootModal.svelte | 89 +++++-- src/lib/helpers/vcsError.test.ts | 73 ++++++ src/lib/helpers/vcsError.ts | 57 +++++ .../functions/create-function/+page.svelte | 43 ++-- .../(modals)/createGit.svelte | 93 ++++++-- .../settings/updateRepository.svelte | 196 ++++++++++++--- .../create-site/repositories/+page.svelte | 95 +++++--- .../createGitDeploymentModal.svelte | 99 ++++++-- .../settings/updateRepository.svelte | 191 ++++++++++++--- 15 files changed, 1268 insertions(+), 287 deletions(-) create mode 100644 src/lib/components/git/installationError.svelte create mode 100644 src/lib/helpers/vcsError.test.ts create mode 100644 src/lib/helpers/vcsError.ts diff --git a/src/lib/components/git/branchSelector.svelte b/src/lib/components/git/branchSelector.svelte index 228a0044c3..db390becae 100644 --- a/src/lib/components/git/branchSelector.svelte +++ b/src/lib/components/git/branchSelector.svelte @@ -9,28 +9,79 @@ import { Query } from '@appwrite.io/console'; import { sdk } from '$lib/stores/sdk'; import { page } from '$app/state'; - import { createEventDispatcher, hasContext, tick } from 'svelte'; - - export let value = ''; - export let installationId: string; - export let repositoryId: string; - export let label = 'Production branch'; - export let placeholder = 'Select branch'; - - const dispatch = createEventDispatcher(); + import { createEventDispatcher, hasContext, tick, untrack } from 'svelte'; + import { getVcsInstallationErrorKind } from '$lib/helpers/vcsError'; + import { installation, installations } from '$lib/stores/vcs'; + import InstallationError from './installationError.svelte'; + + type Props = { + value?: string; + installationId: string; + repositoryId: string; + label?: string; + placeholder?: string; + /** + * Set to `false` when the surrounding surface already renders its own + * installation alert for the same installation, so the user is not shown + * the same "Reconnect" warning twice. The in-list error state stays + * either way: the branch list must never read as "no branches". + */ + showInstallationError?: boolean; + }; + + let { + value = $bindable(''), + installationId, + repositoryId, + label = 'Production branch', + placeholder = 'Select branch', + showInstallationError = true + }: Props = $props(); + + // Deprecated in Svelte 5 but kept deliberately: every call site listens with + // `on:select`, and none of them are in scope here. + const dispatch = createEventDispatcher<{ select: string }>(); const inDialogGroup = hasContext('dialog-group'); - let open = false; - let searchQuery = ''; - let branches: string[] = []; - let searchResults: string[] = []; - let loading = false; - let loaded = false; - let searching = false; + let open = $state(false); + let searchQuery = $state(''); + let branches = $state([]); + let searchResults = $state([]); + let loading = $state(false); + let searching = $state(false); + /** + * The last failure from the branch endpoints. Without it a dead installation + * token renders as a successful empty list ("No branches available"), which + * is the opposite of what happened. + */ + let error = $state(null); + /** Repository the cached `branches` belong to, or `null` when nothing loaded. */ + let loadedFor = $state(null); let searchTimer: ReturnType; - let searchInput: HTMLInputElement; - let containerEl: HTMLDivElement; - let dropdownRect = { top: 0, left: 0, width: 0 }; + let searchInput = $state(); + let containerEl = $state(); + let dropdownRect = $state({ top: 0, left: 0, width: 0 }); + + const repositoryKey = $derived(`${installationId}:${repositoryId}`); + const errorKind = $derived(getVcsInstallationErrorKind(error)); + const displayBranches = $derived(searchQuery ? searchResults : branches); + + /** + * Provider and owner for the reconnect alert. Every route that renders a + * branch selector loads `page.data.installations`; the writable store covers + * the surfaces that pick an installation client side. + */ + const installationDetails = $derived( + $installations?.installations?.find((entry) => entry.$id === installationId) ?? + ($installation?.$id === installationId ? $installation : undefined) + ); + + /** + * When the list could not be loaded the typed query is the only way left to + * name a branch, so it is offered as a selectable value. Only in the error + * state: with a working list, committing an unverified name would be wrong. + */ + const typedBranch = $derived(error ? searchQuery.trim() : ''); function portal(node: HTMLElement) { const target = inDialogGroup ? document.querySelector('dialog[open]') : document.body; @@ -48,16 +99,26 @@ dropdownRect = { top: rect.bottom + 4, left: rect.left, width: rect.width }; } - $: (installationId, - repositoryId, - (() => { + $effect(() => { + // Cached branches and any recorded failure belong to the repository they + // were loaded for, so a change to either id throws them away. Only the + // key is tracked: reading `loadedFor` reactively would re-run this on + // every load and wipe the failure it had just recorded. + const key = repositoryKey; + untrack(() => { + if (loadedFor === key) return; branches = []; - loaded = false; - })()); - - async function loadBranches() { - if (loading || loaded || !installationId || !repositoryId) return; + searchResults = []; + error = null; + loadedFor = null; + }); + }); + + async function loadBranches(force = false) { + const key = repositoryKey; + if (loading || (!force && loadedFor === key) || !installationId || !repositoryId) return; loading = true; + error = null; try { const { branches: result } = await sdk .forProject(page.params.region, page.params.project) @@ -67,7 +128,11 @@ queries: [Query.limit(100)] }); branches = result.map((b) => b.name); - loaded = true; + loadedFor = key; + } catch (e) { + error = e; + branches = []; + loadedFor = null; } finally { loading = false; } @@ -77,6 +142,14 @@ if (!query) { searchResults = []; searching = false; + // A failed search must not outlive the query that caused it. When + // the base list never loaded there is nothing underneath to fall + // back to, so fetch it rather than showing a bare empty list. + if (loadedFor === repositoryKey) { + error = null; + } else { + loadBranches(); + } return; } searching = true; @@ -90,6 +163,12 @@ queries: [Query.limit(100)] }); searchResults = results.map((b) => b.name); + // Cleared on success only, so a failed load keeps explaining itself + // (and keeps the typed value selectable) while the search is running. + error = null; + } catch (e) { + error = e; + searchResults = []; } finally { searching = false; } @@ -100,11 +179,40 @@ searchTimer = setTimeout(() => searchBranches(searchQuery), 300); } + function onSearchKeydown(event: KeyboardEvent) { + if (event.key !== 'Enter' || !typedBranch) return; + event.preventDefault(); + select(typedBranch); + } + + function clearSearch() { + clearTimeout(searchTimer); + searchQuery = ''; + searchResults = []; + searching = false; + // Same reasoning as searchBranches, but only while the dropdown is + // open: the close paths call this too, and reloading then is wasted. + if (loadedFor === repositoryKey) { + error = null; + } else if (open) { + loadBranches(); + } + } + + function retry() { + error = null; + if (searchQuery) { + clearTimeout(searchTimer); + searchBranches(searchQuery); + return; + } + loadBranches(true); + } + function select(branch: string) { value = branch; open = false; - searchQuery = ''; - searchResults = []; + clearSearch(); dispatch('select', branch); } @@ -116,44 +224,47 @@ await tick(); searchInput?.focus(); } else { - searchQuery = ''; - searchResults = []; + clearSearch(); } } function handleKeydown(e: KeyboardEvent) { if (e.key === 'Escape') { open = false; - searchQuery = ''; - searchResults = []; + clearSearch(); } } function handleOutsideClick(e: MouseEvent) { - if (open && !containerEl.contains(e.target as Node)) { + if (open && !containerEl?.contains(e.target as Node)) { const dropdown = document.querySelector('.branch-selector-portal'); if (dropdown && dropdown.contains(e.target as Node)) return; open = false; - searchQuery = ''; - searchResults = []; + clearSearch(); } } - - $: displayBranches = searchQuery ? searchResults : branches; - +
{#if label} - + {/if} - + {#if errorKind && showInstallationError} + + {/if} + {#if open}