Skip to content

fix(vcs): surface invalid installation tokens instead of empty results - #3147

Open
HarshMN2345 wants to merge 2 commits into
mainfrom
feat/vcs-installation-token-errors
Open

fix(vcs): surface invalid installation tokens instead of empty results#3147
HarshMN2345 wants to merge 2 commits into
mainfrom
feat/vcs-installation-token-errors

Conversation

@HarshMN2345

@HarshMN2345 HarshMN2345 commented Aug 4, 2026

Copy link
Copy Markdown
Member

Problem

When a VCS installation's OAuth token can no longer be refreshed, the API fails every repository / branch / contents call with general_provider_failure (Appwrite\Vcs\InstallationTokens). None of the Git surfaces read that error, so a dead installation was presented as a successful empty result:

Surface What the user saw
repositories.svelte loadRepositories() had no try/catch at all, so the empty state claimed the org has no repositories. Used by 7 call sites
branchSelector.svelte, productionBranchFieldset.svelte No error field at all: "No branches available", and the production branch fieldset stuck on its loading state forever
selectRootModal.svelte A spinner that never resolved, or an empty tree reading as "this repository has no directories"
Both updateRepository.svelte cards Fell back to "no repository connected" for a repository that is still connected, inviting the user to reconnect the repo when the installation is the problem
connectRepoModal, the create-deployment modals Empty repo list plus a "Missing a repository? check your permissions" hint that blames scope

Change

src/lib/helpers/vcsError.ts classifies the failure into three kinds, because the remediation genuinely differs:

  • reconnect - the token is dead, only re-authorizing helps
  • locked (general_resource_locked) - a concurrent refresh holds the lock, retrying works. Deliberately never offers a reconnect: the installation is fine
  • provider - the provider itself failed, retry first, reconnect demoted to secondary

src/lib/components/git/installationError.svelte renders all three with Alert.Inline and the reconnect link from the existing $lib/stores/git helpers. Reconnecting is a redirect to the existing authorize URL; the callback upserts the same installation row, so it is repaired in place.

Notable details:

  • The repository cards gain an explicit fourth state rather than overloading the existing null sentinel, which already means "no repository connected" and is exactly the state being confused today.
  • createGitDeploymentModal.svelte and (modals)/createGit.svelte are migrated Svelte 4 -> runes, per AGENTS.md, since they had to be touched. The {#await load()} in both was replaced with an explicit effect-driven load, which also avoids the runes hazard where {#await} re-evaluates when a signal read inside it changes.
  • Modals whose primary action creates a deployment now disable that action on a non-null kind, since the call cannot succeed.

Notes for review

  • The message substring is the only discriminator. general_provider_failure covers both a dead token and a genuine provider outage; only the backend's "Please reconnect it." wording separates them. If reworded, this degrades to the provider branch, which still shows a real error and still offers a reconnect.
  • GitLab reconnect is not reachable from this console. $lib/stores/git exports connectGitHub and connectGitea but no connectGitLab, so the CTA is scoped to the providers the store supports.
  • Detection is reactive only. listInstallations never refreshes tokens, so a broken installation still looks healthy in the installations table until you use it.

Unrelated backend finding, noted in passing

Not a blocker for this PR, and not introduced by it. Recording it for whoever owns VCS.

In appwrite, GitHub/Callback/Get.php initialises the token variables to null and only fills them inside if (!empty($code)), then writes them over the stored credentials unconditionally on the update path. code is an optional param defaulting to '' and is never checked before that write, and the three token attributes are required => false, default => null in platform.php, so a null write succeeds silently rather than erroring. It then fails quietly afterwards, because isExpired(null) returns false.

If GitHub can return a callback with installation_id and no code (an App without "Request user authorization (OAuth) during installation", or a setup_action=update repository-access reconfigure), that blanks working credentials. That is a GitHub behaviour I have not verified, so treat the reachability as unconfirmed.

Worth stressing: this is reachable today through the existing "Configure" / "check your permissions" links, which use the same authorize and callback flow. This PR adds another entry point to it, it does not create it. GitLab and Gitea are unaffected, since their callback requires a code.

Verification

  • bun run check: 0 errors, 87 warnings - byte-identical to the baseline on main
  • vitest src/lib/helpers/vcsError.test.ts: 7/7 pass
  • eslint and prettier --check clean on every changed file

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.
@HarshMN2345
HarshMN2345 marked this pull request as ready for review August 4, 2026 12:52
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR classifies installation-level VCS failures and surfaces actionable retry or reconnect states instead of misleading empty repository, branch, and directory views.

  • Adds shared VCS error classification and installation-error presentation.
  • Propagates installation failures through repository selectors, branch controls, deployment modals, and Function/Site repository settings.
  • Migrates touched deployment and repository surfaces to Svelte 5 runes and explicit asynchronous loading state.

Confidence Score: 4/5

The PR does not yet appear safe to merge because stale branch responses and retained repository rows can still cross repository or installation boundaries.

Branch requests still commit results without checking the current repository key, and unclassified repository-list failures preserve cached rows without verifying that they belong to the selected installation; both previously reported state-isolation failures therefore remain reachable.

Files Needing Attention: src/lib/components/git/branchSelector.svelte; src/lib/components/git/repositories.svelte

Important Files Changed

Filename Overview
src/lib/helpers/vcsError.ts Adds focused classification for locked, reconnect-required, and generic provider failures.
src/lib/components/git/installationError.svelte Adds shared retry and permission-aware reconnect messaging for installation failures.
src/lib/components/git/branchSelector.svelte Adds explicit loading and error handling, but the previously reported stale asynchronous response issue remains.
src/lib/components/git/repositories.svelte Surfaces classified installation failures, but the previously reported cross-installation stale repository path remains for unclassified failures.
src/lib/components/git/selectRootModal.svelte Distinguishes failed content loading from an empty directory tree and prevents saving before the root loads.
src/lib/components/git/productionBranchFieldset.svelte Replaces implicit await rendering with explicit default-branch loading, fallback, and retry states.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateRepository.svelte Separates connected-repository lookup failures from the disconnected state.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRepository.svelte Separates connected-repository lookup failures from the disconnected state.

Reviews (2): Last reviewed commit: "refactor(vcs): trim comments to this rep..." | Re-trigger Greptile

Comment thread src/lib/components/git/branchSelector.svelte
Comment thread src/lib/components/git/repositories.svelte
vcsError.ts carried 43 lines of comment over 14 lines of code, saying the same
thing in three overlapping blocks; the git components it sits beside average
close to none. Kept only what is not inferable from the code: why the message
substring is the discriminator, where that backend copy lives, and why the cast
is structural rather than instanceof.

installationError.svelte's header repeated the same context again.

Same-directory imports of installationError were written two ways across the
files this touched. Now all relative, matching connectGit.svelte.
@HarshMN2345
HarshMN2345 force-pushed the feat/vcs-installation-token-errors branch from 57db238 to d055808 Compare August 4, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant