Skip to content

fix(ui): stop provider delete from surfacing a false error toast #844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skevetter merged 3 commits into from
Aug 1, 2026
Merged

fix(ui): stop provider delete from surfacing a false error toast #844

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
225120f
fix(ui): stop provider delete from surfacing a false error toast
skevetter Aug 1, 2026
e386cf8
fix(ui): snapshot provider name in remaining async provider handlers
skevetter Aug 1, 2026
55f5948
Merge remote-tracking branch 'origin/main' into fix/provider-delete-race
skevetter Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 29 additions & 19 deletions desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ let updating = $state(false)
let confirmSwitchOpen = $state(false)
let targetTag = $state("")
let switching = $state(false)
let status = $derived(providerStatus(provider, $providerJobs[provider.name]))
let status = $derived(
provider
? providerStatus(provider, $providerJobs[provider.name])
: { kind: "uninitialized" as const, label: "" },
)

function openVersionSwitch(tag: string) {
targetTag = tag
Expand All @@ -91,7 +95,9 @@ let hasUnfilledRequired = $derived.by(() => {
return requiredOptions.some(([key]) => !optionValues[key]?.trim())
})

let versionEntry = $derived($providerVersions.byProvider[provider.name])
let versionEntry = $derived(
provider ? $providerVersions.byProvider[provider.name] : undefined,
)
Comment on lines +98 to +100

@coderabbitai coderabbitai Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Complete null safety for in-flight handlers.

The component still declares provider: Provider at Line 50, although this PR allows provider to become undefined. The conditional markup does not cancel handlers that started earlier. If the provider disappears during an await, runUpdate, runSwitch, handleSetDefault, and handleInitialize can still dereference provider.name, causing false error toasts or failed cleanup.

Change the prop type to Provider | undefined. Snapshot the provider name before each asynchronous flow and use the local name after every await.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte` around
lines 96 - 98, Update the ProviderSheet provider prop type from Provider to
Provider | undefined, then in runUpdate, runSwitch, handleSetDefault, and
handleInitialize capture provider.name in a local variable before each
asynchronous flow and use that snapshot after every await. Guard the initial
access when provider is absent so in-flight handlers cannot dereference the
mutable provider value or trigger incorrect cleanup and error handling.


let groupedOptions = $derived.by(() => {
const groups: Record<string, [string, ProviderOption][]> = {}
Expand Down Expand Up @@ -135,7 +141,7 @@ async function loadOptions() {
}

$effect(() => {
if (!open) {
if (!open || !provider) {
loadedFor = null
return
}
Expand All @@ -147,9 +153,10 @@ $effect(() => {
})

async function handleSetDefault() {
const name = provider.name
try {
await providerUse(provider.name)
toasts.success(`Set ${provider.name} as default provider`)
await providerUse(name)
toasts.success(`Set ${name} as default provider`)
} catch (err) {
toasts.error(`Failed to set default: ${extractErrorMessage(err)}`)
}
Expand Down Expand Up @@ -177,14 +184,15 @@ function handleUpdate() {
}

async function runUpdate() {
const name = provider.name
updating = true
try {
// Also re-initializes: the new binaries have not run their init, and
// set-source clears the initialized flag accordingly.
await providerUpdate(provider.name)
toasts.success(`Updated ${provider.name}`)
await providerUpdate(name)
toasts.success(`Updated ${name}`)
providers.set(await providerList())
await loadVersionsFor(provider.name)
await loadVersionsFor(name)
await refreshUpdates()
} catch (err) {
toasts.error(`Failed to update: ${extractErrorMessage(err)}`)
Expand All @@ -195,12 +203,13 @@ async function runUpdate() {
}

async function runSwitch() {
const name = provider.name
switching = true
try {
await providerSetVersion(provider.name, targetTag)
toasts.success(`Switched ${provider.name} to ${targetTag}`)
await providerSetVersion(name, targetTag)
toasts.success(`Switched ${name} to ${targetTag}`)
providers.set(await providerList())
await loadVersionsFor(provider.name)
await loadVersionsFor(name)
await refreshUpdates()
} catch (err) {
toasts.error(`Failed to switch version: ${extractErrorMessage(err)}`)
Expand All @@ -227,24 +236,22 @@ function extractCliError(err: unknown): CLIError | null {
}

async function handleInitialize() {
const name = provider.name
initializing = true
initError = null
try {
await providerInit(provider.name)
await providerInit(name)
const updated = await providerList()
providers.set(updated)
toasts.success(`Initialized ${provider.name}`)
toasts.success(`Initialized ${name}`)
} catch (err) {
const cliError = extractCliError(err)
if (cliError) {
initError = cliError
} else {
initError = {
code: "UNKNOWN",
message:
err instanceof Error
? err.message
: `Failed to initialize ${provider.name}.`,
message: err instanceof Error ? err.message : `Failed to initialize ${name}.`,
}
}
} finally {
Expand All @@ -253,10 +260,11 @@ async function handleInitialize() {
}

async function handleDelete() {
const name = provider.name
deleting = true
try {
await providerDelete(provider.name)
toasts.success(`Deleted ${provider.name}`)
await providerDelete(name)
toasts.success(`Deleted ${name}`)
confirmDeleteOpen = false
open = false
ondeleted?.()
Expand Down Expand Up @@ -318,6 +326,7 @@ async function handleSaveOptions() {
}
</script>

{#if provider}
<Sheet.Root bind:open>
<Sheet.ResizableContent>
<Sheet.Header class="p-6">
Expand Down Expand Up @@ -537,3 +546,4 @@ async function handleSaveOptions() {
loading={switching}
onconfirm={runSwitch}
/>
{/if}
Loading