From 240672042d732b55f278ee02fcadb93d40b0a37f Mon Sep 17 00:00:00 2001 From: estib Date: Mon, 28 Oct 2024 10:35:33 +0100 Subject: [PATCH] fix: Mitigate Prompt modal race condition Add some checks and await the handling of git prompts. --- apps/desktop/src/lib/backend/prompt.ts | 45 +++++++++++++------ .../src/lib/components/PromptModal.svelte | 38 +++++++++------- 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/apps/desktop/src/lib/backend/prompt.ts b/apps/desktop/src/lib/backend/prompt.ts index ce2923d2f6..bc38e38a45 100644 --- a/apps/desktop/src/lib/backend/prompt.ts +++ b/apps/desktop/src/lib/backend/prompt.ts @@ -25,12 +25,40 @@ type FilterParams = { export type SystemPromptHandle = { prompt: string; - respond: (response: string | null) => void; + respond: (response: string | null) => Promise; }; type PromptHandler = (prompt: SystemPrompt, signal: AbortSignal) => Promise; type FilterEventEntry = [filter: FilterParams, handler: PromptHandler]; +/** + * Handle the abort signal for a prompt handler. + * + * This is required to be able to await the handling of a prompt. + */ +async function handleAbortSignal( + signal: AbortSignal, + promptStore: Writable, + errorStore: Writable +): Promise { + const signalHandler = new Promise((resolve) => { + signal.addEventListener('abort', () => { + promptStore.set(undefined); + switch (signal.reason) { + case 'timeout': + errorStore.set('Timed out waiting for response'); + break; + default: + errorStore.set(undefined); + break; + } + resolve(); + }); + }); + + await signalHandler; +} + /** * This service is used for handling CLI prompts from the back end. */ @@ -127,20 +155,9 @@ export class PromptService { promptStore.set({ prompt: prompt.prompt, - respond: (response: string | null) => { + respond: async (response: string | null) => { resolver(response); - } - }); - - signal.addEventListener('abort', () => { - promptStore.set(undefined); - switch (signal.reason) { - case 'timeout': - errorStore.set('Timed out waiting for response'); - break; - default: - errorStore.set(undefined); - break; + await handleAbortSignal(signal, promptStore, errorStore); } }); diff --git a/apps/desktop/src/lib/components/PromptModal.svelte b/apps/desktop/src/lib/components/PromptModal.svelte index 3041491c12..4bcef9b644 100644 --- a/apps/desktop/src/lib/components/PromptModal.svelte +++ b/apps/desktop/src/lib/components/PromptModal.svelte @@ -8,23 +8,22 @@ const promptService = getContext(PromptService); const [prompt, error] = promptService.reactToPrompt({ timeoutMs: 30000 }); - let value = ''; - let modal: Modal; - let loading = false; + let value = $state(''); + let modal = $state>(); + let loading = $state(false); - $: if ($prompt) { - modal?.show(); - } - - $: if (!$prompt && !$error) { - modal?.close(); - } + $effect(() => { + if ($prompt && modal?.imports.open === false && !loading) { + modal?.show(); + } + }); async function submit() { if (!$prompt) return; loading = true; try { - $prompt.respond(value); + await modal?.close(); + await $prompt.respond(value); } catch (err) { console.error(err); } finally { @@ -35,7 +34,7 @@ async function cancel() { try { - if ($prompt) $prompt.respond(null); + if ($prompt) await $prompt.respond(null); } catch (err) { console.error(err); } finally { @@ -43,6 +42,11 @@ } } + async function handleCancelButton() { + await modal?.close(); + await cancel(); + } + function clear() { prompt.set(undefined); error.set(undefined); @@ -53,9 +57,9 @@ await cancel()} - onSubmit={async () => await submit()} + title="Git needs input" + onClickOutside={cancel} + onSubmit={submit} >
{#if $error} @@ -67,7 +71,9 @@ {#snippet controls()} - +