From b1f5f4ebaf2305bbd32b7928573ff6336196cfe9 Mon Sep 17 00:00:00 2001 From: HackedRico Date: Mon, 31 Aug 2026 10:59:21 -0400 Subject: [PATCH 1/2] fix(chat): keep Author and Plan and Execute runs alive when leaving the page Starting a run locked the whole workflow page. Back, the History toggle and New chat were disabled until it finished, so a run could not be sent to the background even though the backend already runs it detached and serves /status from an in-memory cache. The guard was load bearing rather than gratuitous: nothing on the client remembered a run once the component unmounted, so removing the :disabled alone would have orphaned the run in the UI while the server kept working. Three pieces make the guards removable: - persist the run_id on its assistant message, written as soon as /execute answers rather than on the first poll, so clicking Back within the first second still keeps the handle - re-attach a poller on mount to the newest message still marked RUNNING, and read its status once up front so the view paints the real state instead of stale content - stop the poller in onBeforeUnmount, which also fixes the 1s interval leaking whenever the component unmounted by any other route hydrate() now only fails a RUNNING message it cannot resume: an older one, or one that never captured a run_id. Those still point at the History tab, as does a re-attach that 404s because the server restarted or the run aged out of the live cache. The composer stays gated while a run is in flight. This view polls one run at a time, and a second prompt would silently orphan the first, so that guard is about concurrency rather than about trapping the user. Its placeholder now says the page can be left. Two supporting fixes in the poller, both reachable now that New chat works mid-run: drop status snapshots for a run the composable has moved on from, which would otherwise strand the view on a RUNNING status nothing polls any more, and ride out transient status failures instead of failing a healthy run on a single blip. Refs #30 --- gui/views/chat/ChatWorkflow.vue | 64 +++++++-- gui/views/chat/composables/useChatSession.js | 57 ++++---- gui/views/chat/composables/useMcpRun.js | 137 ++++++++++++++----- 3 files changed, 187 insertions(+), 71 deletions(-) diff --git a/gui/views/chat/ChatWorkflow.vue b/gui/views/chat/ChatWorkflow.vue index bc738a5..707ea2f 100644 --- a/gui/views/chat/ChatWorkflow.vue +++ b/gui/views/chat/ChatWorkflow.vue @@ -2,7 +2,13 @@ Top-level chat-style page for a single MCP workflow. Owns the message transcript and a single in-flight run; each user prompt triggers one /plugin/mcp/execute call and appends the resulting assistant message to - the transcript. The composer is disabled while a run is in progress. + the transcript. + + A run belongs to the server, not to this component: leaving the page only + stops the poller. The run_id rides along on its assistant message, so a + remount re-attaches to a run that is still going. Navigation is never + blocked; only the composer is gated while a run is in flight, because the + view polls one run at a time. This component intentionally does not maintain a server-side session: the backend is still single-shot. The chat UI gives users a familiar Claude.ai @@ -25,15 +31,16 @@ class="back-button" @click="$emit('back')" type="button" - :disabled="run.isRunning.value" - :title="run.isRunning.value - ? 'Cannot leave while a run is in progress' - : 'Back to workflow list'" + title="Back to workflow list" > Back - + Working @@ -48,7 +55,6 @@ class="header-action history-toggle" :class="{ 'is-on': historyEnabled }" @click="historyEnabled = !historyEnabled" - :disabled="run.isRunning.value" type="button" :title="historyEnabled ? 'Chat history is being threaded into each prompt. Click to disable for the rest of this session.' @@ -61,9 +67,10 @@ v-if="messages.length" class="header-action" @click="clearTranscript" - :disabled="run.isRunning.value" type="button" - title="Start a new chat" + :title="run.isRunning.value + ? 'Start a new chat. The run in progress keeps going on the server and stays in the History tab.' + : 'Start a new chat'" > New chat @@ -183,6 +190,9 @@ onMounted(() => { // before the first render pass. Has to happen before the height sync so // the transcript scroll position settles correctly on a hydrated view. session.hydrate() + // A run that was in flight at the last unmount is still going server side, + // and hydrate() leaves its bubble RUNNING, so give it a fresh poller. + _resumeRunInFlight() prevBodyOverflow = document.body.style.overflow prevHtmlOverflow = document.documentElement.style.overflow @@ -208,9 +218,14 @@ const { thoughts, adversary, abilityNames, splitSentences, isInjectedSentence } // Sync the in-flight run into its assistant message as state changes. The // assistant message is created at submit time with a known id; we update // that message in place rather than re-pushing. +// +// runId is watched alongside the run state so the id reaches the persisted +// transcript as soon as /execute answers. Waiting for the first poll would +// leave a one-second window where clicking Back loses the handle on the run. let pendingAssistantId = null watch( () => ({ + runId: run.runId.value, status: run.status.value, stage: run.stage.value, finalResult: run.finalResult.value, @@ -222,6 +237,7 @@ watch( if (!pendingAssistantId) return const msg = messages.value.find(m => m.id === pendingAssistantId) if (!msg) return + msg.runId = run.runId.value msg.status = run.status.value msg.stage = run.stage.value msg.finalResult = run.finalResult.value @@ -237,10 +253,30 @@ watch( { deep: true } ) +// Re-attach to a run that outlived the last unmount. Only the newest +// assistant bubble can still own one: hydrate() has already failed every +// older RUNNING message, and the composer admits one run at a time. +function _resumeRunInFlight() { + const pending = [...messages.value] + .reverse() + .find(m => m.role === 'assistant' && m.status === 'RUNNING' && m.runId) + if (!pending) return + pendingAssistantId = pending.id + run.attach(pending.runId) +} + +// Drop the poller on the way out. The run keeps going on the server and its +// bubble stays RUNNING in localStorage, which is what _resumeRunInFlight +// picks up on the next mount. Without this the 1s interval outlives the view. +onBeforeUnmount(run.stop) + // --- Workflow-derived UI bits ---------------------------------------------- const examplePrompts = computed(() => props.workflow?.example_prompts || []) const composerPlaceholder = computed(() => { - if (run.isRunning.value) return 'Working on the previous request…' + // Says why the box is closed, and that closing it is not a page lock: the + // view polls one run at a time, but the run does not need the page open. + if (run.isRunning.value) + return 'Working on the previous request. You can leave this page, it keeps running…' if (messages.value.length) { if (historyActive.value) return 'Send a follow-up prompt in this session…' if (supportsChatHistory.value) @@ -269,6 +305,9 @@ function handleSubmit() { messages.value.push({ id: assistantId, role: 'assistant', + // Filled in by the run watcher once /execute answers. It is the handle + // a later mount uses to re-attach, so it is part of the stored message. + runId: null, status: 'RUNNING', stage: '', finalResult: '', @@ -376,7 +415,10 @@ async function _startRun(text) { } function clearTranscript() { - if (run.isRunning.value) return + // Allowed mid-run: run.reset() drops the poller, and the run itself carries + // on server side and stays reachable in the History tab. What it loses is + // this view's link to it, which is the point of starting a new chat. + // // "New chat" is the only path that wipes durable session state. The // composable resets messages, sessionId, historyEnabled, and // selectedRag in memory AND removes this workflow's slice from diff --git a/gui/views/chat/composables/useChatSession.js b/gui/views/chat/composables/useChatSession.js index 07cbf2e..93a5e44 100644 --- a/gui/views/chat/composables/useChatSession.js +++ b/gui/views/chat/composables/useChatSession.js @@ -23,9 +23,10 @@ // What it does not own: // composerText, transient run-state (status/stage/finalResult on the // in-flight assistant message). Composer text is intentionally not -// persisted — half-typed prompts surviving a remount is a worse UX -// than a clean text box. In-flight run state belongs to useMcpRun -// and is reconstructed from the polling endpoint, not localStorage. +// persisted: half-typed prompts surviving a remount is a worse UX +// than a clean text box. In-flight run state belongs to useMcpRun and +// is reconstructed from the polling endpoint; localStorage only keeps +// the run_id needed to ask for it again. // // Storage shape: // localStorage key 'mcp_chat_sessions' holds: @@ -36,11 +37,12 @@ // The schema version lets future changes drop incompatible blobs // without confusing rehydration. // -// On hydrate the composable also marks any message whose status is -// still 'RUNNING' as FAILED with a "polling stopped" note. Polling -// timers do not survive a component unmount; leaving the bubble -// frozen on "Thinking" forever is misleading. The MLflow run is still -// findable via the History tab if the user needs the result. +// Assistant messages carry the run_id of the run that produced them, so +// a bubble left mid-run survives a remount: the server keeps working and +// ChatWorkflow re-attaches a poller to the newest RUNNING message. Older +// RUNNING messages, and any that never captured a run_id, are marked +// FAILED on hydrate with a pointer to the History tab, since nothing is +// going to move them again. // // Limits: // Per-workflow caps at MAX_MESSAGES messages and MAX_BYTES total @@ -101,24 +103,27 @@ function _trimMessages(messages) { return trimmed } -function _markStaleRunningAsFailed(messages) { - // A polling timer cannot survive a component unmount. Any message - // that was RUNNING when we last saved is no longer being updated; - // surfacing the stale state would lie to the user. Mark it FAILED - // with a hint so the user knows where to look (History tab). - return messages.map(m => { - if (m.status === 'RUNNING') { - return { - ...m, - status: 'FAILED', - stage: '', - errorMessage: - m.errorMessage - || 'Polling stopped when you navigated away. ' - + 'Check the History tab for the final result.', - } +function _markUnresumableRunning(messages) { + // The server keeps running after this view unmounts, so the newest + // RUNNING message can be re-attached by its runId. Anything older lost + // its poller for good (the view polls one run at a time), as did any + // message that never captured a runId, and a bubble left frozen on + // "Thinking" would lie to the user. Point those at the History tab. + const resumableIndex = messages.reduce( + (last, m, i) => (m.status === 'RUNNING' && m.runId ? i : last), + -1 + ) + return messages.map((m, i) => { + if (m.status !== 'RUNNING' || i === resumableIndex) return m + return { + ...m, + status: 'FAILED', + stage: '', + errorMessage: + m.errorMessage + || 'The page stopped tracking this run. ' + + 'Check the History tab for the final result.', } - return m }) } @@ -131,7 +136,7 @@ export function useChatSession(workflowId) { function hydrate() { const slice = _readWorkflow(workflowId) if (!slice) return - messages.value = _markStaleRunningAsFailed(slice.messages || []) + messages.value = _markUnresumableRunning(slice.messages || []) sessionId.value = slice.sessionId ?? null historyEnabled.value = typeof slice.historyEnabled === 'boolean' ? slice.historyEnabled : true diff --git a/gui/views/chat/composables/useMcpRun.js b/gui/views/chat/composables/useMcpRun.js index cbb9d94..6059096 100644 --- a/gui/views/chat/composables/useMcpRun.js +++ b/gui/views/chat/composables/useMcpRun.js @@ -1,13 +1,23 @@ // One MCP run: POST /plugin/mcp/execute, then poll /plugin/mcp/status until -// FINISHED or FAILED. Returns reactive state plus a start() function. +// FINISHED or FAILED. Returns reactive state plus start() and attach(). // -// Each run is independent — start() resets all state and begins fresh. The +// Each run is independent. start() resets all state and begins fresh; the // caller decides how to surface results (e.g. push into a chat transcript). +// +// The run itself belongs to the server: /execute hands back a run_id right +// away and the workflow keeps going no matter what the browser does. attach() +// is the counterpart to start() for a run that is already in flight, which is +// how a remounted view picks its run back up after the user navigated away. import { ref, computed } from 'vue' const POLL_INTERVAL_MS = 1000 +// One failed status GET is usually a blip (CALDERA busy on the event loop it +// serves every plugin from), not a dead run. Only give up after this many in +// a row so a hiccup does not report a healthy run as FAILED. +const MAX_CONSECUTIVE_POLL_ERRORS = 3 + export function useMcpRun($api) { const status = ref('idle') // 'idle' | 'RUNNING' | 'FINISHED' | 'FAILED' const stage = ref('') @@ -23,6 +33,17 @@ export function useMcpRun($api) { const isFailed = computed(() => status.value === 'FAILED') let pollTimer = null + let consecutivePollErrors = 0 + // Set by stop() so a status GET that is already in flight cannot start a + // fresh interval after the caller has walked away (unmount). + let detached = false + + function _clearTimer() { + if (pollTimer) { + clearInterval(pollTimer) + pollTimer = null + } + } function reset() { status.value = 'idle' @@ -33,12 +54,12 @@ export function useMcpRun($api) { finalResult.value = '' trajectory.value = {} errorMessage.value = '' - if (pollTimer) { - clearInterval(pollTimer) - pollTimer = null - } + consecutivePollErrors = 0 + detached = false + _clearTimer() } + /** Submit a new run and begin polling it; resolves with the /execute body. */ async function start(payload) { reset() status.value = 'RUNNING' @@ -58,46 +79,94 @@ export function useMcpRun($api) { } } + /** Resume polling a run already in flight on the server, by its `run_id`. */ + async function attach(id) { + reset() + status.value = 'RUNNING' + runId.value = id + // Read the run once up front so a remounted view paints its real state + // straight away instead of sitting on stale content for a whole interval. + const stillLive = await _pollOnce(id) + if (stillLive && !detached) _beginPolling(id) + } + + function _applySnapshot(data) { + status.value = data.status || 'unknown' + stage.value = data.stage || '' + prompt.value = data.prompt || prompt.value + reasoning.value = data.reasoning || '' + finalResult.value = data.process_result || '' + trajectory.value = data.trajectory || {} + // The backend writes a per-run `error` field into the run cache when + // the workflow raises (see mcp_svc._run_execution's except branch). + // Surface it so ChatMessage can render the actual cause under the + // "Run failed." headline instead of leaving it blank. + if (data.error) errorMessage.value = data.error + } + + // True once this composable has moved on from `id`, which reset() signals by + // clearing runId. "New chat" resets mid-run, so a GET issued for the run the + // user just walked away from can still be in flight; letting it land would + // strand the view on a RUNNING status nothing polls any more. + function _superseded(id) { + return runId.value !== id + } + + // Pull one status snapshot into the reactive state. Returns false once + // polling should stop, either because the run reached a terminal state or + // because the endpoint will not answer for this run again. + async function _pollOnce(id) { + try { + const res = await $api.get('/plugin/mcp/status', { params: { run_id: id } }) + if (_superseded(id)) return false + consecutivePollErrors = 0 + _applySnapshot(res.data) + return status.value !== 'FINISHED' && status.value !== 'FAILED' + } catch (err) { + if (_superseded(id)) return false + // 404 means the run left the live cache: the server restarted, or the + // run aged out of the LRU bound. Neither resolves by asking again. + if (err?.response?.status === 404) { + status.value = 'FAILED' + errorMessage.value = + 'This run is no longer tracked live. ' + + 'Check the History tab for its result.' + return false + } + consecutivePollErrors += 1 + if (consecutivePollErrors < MAX_CONSECUTIVE_POLL_ERRORS) return true + status.value = 'FAILED' + errorMessage.value = 'Polling failed.' + return false + } + } + function _beginPolling(id) { - if (pollTimer) clearInterval(pollTimer) + _clearTimer() + // Scoped per polling session so a slow status GET cannot stack up behind + // itself when the server takes longer than one interval to answer. + let inFlight = false pollTimer = setInterval(async () => { + if (inFlight) return + inFlight = true try { - const res = await $api.get('/plugin/mcp/status', { params: { run_id: id } }) - status.value = res.data.status || 'unknown' - stage.value = res.data.stage || '' - prompt.value = res.data.prompt || prompt.value - reasoning.value = res.data.reasoning || '' - finalResult.value = res.data.process_result || '' - trajectory.value = res.data.trajectory || {} - // The backend writes a per-run `error` field into the run cache when - // the workflow raises (see mcp_svc._run_execution's except branch). - // Surface it so ChatMessage can render the actual cause under the - // "Run failed." headline instead of leaving it blank. - if (res.data.error) errorMessage.value = res.data.error - - if (status.value === 'FINISHED' || status.value === 'FAILED') { - clearInterval(pollTimer) - pollTimer = null - } - } catch (e) { - clearInterval(pollTimer) - pollTimer = null - status.value = 'FAILED' - errorMessage.value = 'Polling failed.' + const stillLive = await _pollOnce(id) + if (!stillLive) _clearTimer() + } finally { + inFlight = false } }, POLL_INTERVAL_MS) } + /** Stop polling without touching run state; the server run keeps going. */ function stop() { - if (pollTimer) { - clearInterval(pollTimer) - pollTimer = null - } + detached = true + _clearTimer() } return { status, stage, runId, prompt, reasoning, finalResult, trajectory, errorMessage, isRunning, isFinished, isFailed, - start, stop, reset, + start, attach, stop, reset, } } From 15e44ea9c8e607dbd6c09cfc4fd90c1d8c5e493f Mon Sep 17 00:00:00 2001 From: HackedRico Date: Tue, 1 Sep 2026 13:40:29 -0400 Subject: [PATCH 2/2] fix(chat): close four races the background-run change opened An adversarial review of b1f5f4e found four client-side races, three of them reachable only because that commit unlocked navigation. start() checked neither supersession nor detachment after its POST. Back mid-POST leaked a poller into an unmounted view, and New chat mid-POST adopted the abandoned run's ids into the transcript that replaced it. The interval callback cleared whichever timer was currently installed rather than its own, so a superseded run whose status GET settled late killed the next run's poller, stranding it on RUNNING with the composer disabled and no in-view recovery. The run_id reached localStorage only through watchers Vue stops on unmount, so clicking Back during the POST stored a live run with runId null and the next mount labelled a working run as failed. It is now written imperatively at the point /execute answers. An expired session arrives as a 200 carrying the login page rather than a snapshot, which was parsed into status "unknown" and persisted. That value is recognised by neither the resume path nor the hydrate sweep, so the bubble could be neither re-attached nor failed. gui/composables already documents this trap; reuse its SESSION_EXPIRED message. Also stop reporting a lost poll as a failed run: three consecutive status errors now say contact was lost and point at the History tab. Refs #30 --- gui/views/chat/ChatWorkflow.vue | 58 +++++++------ gui/views/chat/composables/useChatSession.js | 20 ++--- gui/views/chat/composables/useMcpRun.js | 88 ++++++++++++-------- 3 files changed, 89 insertions(+), 77 deletions(-) diff --git a/gui/views/chat/ChatWorkflow.vue b/gui/views/chat/ChatWorkflow.vue index 707ea2f..5760afc 100644 --- a/gui/views/chat/ChatWorkflow.vue +++ b/gui/views/chat/ChatWorkflow.vue @@ -4,11 +4,9 @@ /plugin/mcp/execute call and appends the resulting assistant message to the transcript. - A run belongs to the server, not to this component: leaving the page only - stops the poller. The run_id rides along on its assistant message, so a - remount re-attaches to a run that is still going. Navigation is never - blocked; only the composer is gated while a run is in flight, because the - view polls one run at a time. + A run belongs to the server, so leaving the page only stops the poller. The + run_id rides on its assistant message, and a remount re-attaches. Only the + composer is gated during a run, because this view polls one at a time. This component intentionally does not maintain a server-side session: the backend is still single-shot. The chat UI gives users a familiar Claude.ai @@ -190,8 +188,7 @@ onMounted(() => { // before the first render pass. Has to happen before the height sync so // the transcript scroll position settles correctly on a hydrated view. session.hydrate() - // A run that was in flight at the last unmount is still going server side, - // and hydrate() leaves its bubble RUNNING, so give it a fresh poller. + // hydrate() leaves a still-running bubble RUNNING; give it a fresh poller. _resumeRunInFlight() prevBodyOverflow = document.body.style.overflow @@ -217,11 +214,8 @@ const { thoughts, adversary, abilityNames, splitSentences, isInjectedSentence } // Sync the in-flight run into its assistant message as state changes. The // assistant message is created at submit time with a known id; we update -// that message in place rather than re-pushing. -// -// runId is watched alongside the run state so the id reaches the persisted -// transcript as soon as /execute answers. Waiting for the first poll would -// leave a one-second window where clicking Back loses the handle on the run. +// that message in place rather than re-pushing. runId is watched too so the +// handle reaches the transcript without waiting for the first poll. let pendingAssistantId = null watch( () => ({ @@ -253,9 +247,8 @@ watch( { deep: true } ) -// Re-attach to a run that outlived the last unmount. Only the newest -// assistant bubble can still own one: hydrate() has already failed every -// older RUNNING message, and the composer admits one run at a time. +// Only the newest bubble can still own a run: hydrate() has already failed +// every older RUNNING message. function _resumeRunInFlight() { const pending = [...messages.value] .reverse() @@ -265,16 +258,13 @@ function _resumeRunInFlight() { run.attach(pending.runId) } -// Drop the poller on the way out. The run keeps going on the server and its -// bubble stays RUNNING in localStorage, which is what _resumeRunInFlight -// picks up on the next mount. Without this the 1s interval outlives the view. +// Without this the 1s interval outlives the view. The run itself keeps going. onBeforeUnmount(run.stop) // --- Workflow-derived UI bits ---------------------------------------------- const examplePrompts = computed(() => props.workflow?.example_prompts || []) const composerPlaceholder = computed(() => { - // Says why the box is closed, and that closing it is not a page lock: the - // view polls one run at a time, but the run does not need the page open. + // The box is closed, but the page is not: say so. if (run.isRunning.value) return 'Working on the previous request. You can leave this page, it keeps running…' if (messages.value.length) { @@ -305,8 +295,7 @@ function handleSubmit() { messages.value.push({ id: assistantId, role: 'assistant', - // Filled in by the run watcher once /execute answers. It is the handle - // a later mount uses to re-attach, so it is part of the stored message. + // Set by _recordRunHandle; the handle a later mount re-attaches with. runId: null, status: 'RUNNING', stage: '', @@ -319,10 +308,20 @@ function handleSubmit() { }) composerText.value = '' - _startRun(text) + _startRun(text, assistantId) +} + +// Written imperatively because Vue stops the watchers on unmount: clicking +// Back mid-POST would otherwise store a live run with runId null, and the +// next mount would call it failed. +function _recordRunHandle(assistantId, resp) { + if (resp.session_id && !sessionId.value) sessionId.value = resp.session_id + const msg = messages.value.find(m => m.id === assistantId) + if (msg) msg.runId = resp.run_id || null + session.persist() } -async function _startRun(text) { +async function _startRun(text, assistantId) { const context = workflowContext.value || {} const selectedCtiFiles = Array.isArray(context.selected_stix_files) ? context.selected_stix_files @@ -406,18 +405,17 @@ async function _startRun(text) { try { const resp = await run.start(payload) - if (resp?.session_id && !sessionId.value) { - sessionId.value = resp.session_id - } + // null means "New chat" wiped this transcript mid-POST; adopting the ids + // would thread the fresh chat onto the run the user walked away from. + if (resp) _recordRunHandle(assistantId, resp) } catch { // run.errorMessage is already populated by useMcpRun. } } function clearTranscript() { - // Allowed mid-run: run.reset() drops the poller, and the run itself carries - // on server side and stays reachable in the History tab. What it loses is - // this view's link to it, which is the point of starting a new chat. + // Allowed mid-run: only this view's link to the run is lost, which is the + // point. The run carries on and stays reachable in the History tab. // // "New chat" is the only path that wipes durable session state. The // composable resets messages, sessionId, historyEnabled, and diff --git a/gui/views/chat/composables/useChatSession.js b/gui/views/chat/composables/useChatSession.js index 93a5e44..c0ced3d 100644 --- a/gui/views/chat/composables/useChatSession.js +++ b/gui/views/chat/composables/useChatSession.js @@ -37,12 +37,10 @@ // The schema version lets future changes drop incompatible blobs // without confusing rehydration. // -// Assistant messages carry the run_id of the run that produced them, so -// a bubble left mid-run survives a remount: the server keeps working and -// ChatWorkflow re-attaches a poller to the newest RUNNING message. Older -// RUNNING messages, and any that never captured a run_id, are marked -// FAILED on hydrate with a pointer to the History tab, since nothing is -// going to move them again. +// Assistant messages carry their run_id, so a bubble left mid-run survives +// a remount and ChatWorkflow re-attaches a poller to it. Older RUNNING +// messages, and any with no run_id, are failed on hydrate: nothing is going +// to move them again. // // Limits: // Per-workflow caps at MAX_MESSAGES messages and MAX_BYTES total @@ -104,11 +102,8 @@ function _trimMessages(messages) { } function _markUnresumableRunning(messages) { - // The server keeps running after this view unmounts, so the newest - // RUNNING message can be re-attached by its runId. Anything older lost - // its poller for good (the view polls one run at a time), as did any - // message that never captured a runId, and a bubble left frozen on - // "Thinking" would lie to the user. Point those at the History tab. + // Only the newest RUNNING message can be re-attached; the view polls one + // run at a time. A bubble left frozen on "Thinking" would lie to the user. const resumableIndex = messages.reduce( (last, m, i) => (m.status === 'RUNNING' && m.runId ? i : last), -1 @@ -177,6 +172,9 @@ export function useChatSession(workflowId) { historyEnabled, selectedRag, hydrate, + // For the one write that cannot wait for the watcher: a run_id arriving + // after unmount, once the scope holding that watcher is stopped. + persist, reset, } } diff --git a/gui/views/chat/composables/useMcpRun.js b/gui/views/chat/composables/useMcpRun.js index 6059096..c3c4e6e 100644 --- a/gui/views/chat/composables/useMcpRun.js +++ b/gui/views/chat/composables/useMcpRun.js @@ -1,21 +1,16 @@ // One MCP run: POST /plugin/mcp/execute, then poll /plugin/mcp/status until // FINISHED or FAILED. Returns reactive state plus start() and attach(). // -// Each run is independent. start() resets all state and begins fresh; the -// caller decides how to surface results (e.g. push into a chat transcript). -// -// The run itself belongs to the server: /execute hands back a run_id right -// away and the workflow keeps going no matter what the browser does. attach() -// is the counterpart to start() for a run that is already in flight, which is -// how a remounted view picks its run back up after the user navigated away. +// The run belongs to the server, so it outlives the browser. attach() is the +// counterpart to start() for one already in flight, which is how a remounted +// view picks its run back up. import { ref, computed } from 'vue' +import { SESSION_EXPIRED } from '../../../composables/request.js' const POLL_INTERVAL_MS = 1000 -// One failed status GET is usually a blip (CALDERA busy on the event loop it -// serves every plugin from), not a dead run. Only give up after this many in -// a row so a hiccup does not report a healthy run as FAILED. +// A single failed status GET is usually a blip, not a dead run. const MAX_CONSECUTIVE_POLL_ERRORS = 3 export function useMcpRun($api) { @@ -34,9 +29,12 @@ export function useMcpRun($api) { let pollTimer = null let consecutivePollErrors = 0 - // Set by stop() so a status GET that is already in flight cannot start a - // fresh interval after the caller has walked away (unmount). + // Set by stop() so a request already in flight cannot start a fresh + // interval after the caller unmounted. let detached = false + // Bumped by reset(). runId cannot mark supersession on its own: it is null + // for the whole /execute POST, which is the window "New chat" opens. + let generation = 0 function _clearTimer() { if (pollTimer) { @@ -56,23 +54,30 @@ export function useMcpRun($api) { errorMessage.value = '' consecutivePollErrors = 0 detached = false + generation += 1 _clearTimer() } - /** Submit a new run and begin polling it; resolves with the /execute body. */ + /** + * Submit a run; resolves with the /execute body, or null if "New chat" + * superseded it mid-POST. An unmounted caller still gets the body so the + * run_id can be persisted; only the polling is skipped. + */ async function start(payload) { reset() + const gen = generation status.value = 'RUNNING' prompt.value = payload.text || '' try { const response = await $api.post('/plugin/mcp/execute', payload) + if (gen !== generation) return null runId.value = response.data.run_id - _beginPolling(runId.value) - // Hand the parsed response back so the caller can grab fields like - // session_id that live above the per-run scope. + if (!detached) _beginPolling(runId.value) + // The caller needs fields above the per-run scope, like session_id. return response.data } catch (err) { + if (gen !== generation) return null status.value = 'FAILED' errorMessage.value = err?.response?.data?.error || 'Submission failed.' throw err @@ -84,12 +89,18 @@ export function useMcpRun($api) { reset() status.value = 'RUNNING' runId.value = id - // Read the run once up front so a remounted view paints its real state - // straight away instead of sitting on stale content for a whole interval. + // Read once up front so a remounted view paints real state immediately + // instead of holding stale content for a whole interval. const stillLive = await _pollOnce(id) if (stillLive && !detached) _beginPolling(id) } + // An expired session lands here as a 200 carrying the login page, not a + // snapshot (see gui/composables/request.js). Never persist that as a status. + function _isSnapshot(data) { + return !!data && typeof data === 'object' && typeof data.status === 'string' + } + function _applySnapshot(data) { status.value = data.status || 'unknown' stage.value = data.stage || '' @@ -97,35 +108,34 @@ export function useMcpRun($api) { reasoning.value = data.reasoning || '' finalResult.value = data.process_result || '' trajectory.value = data.trajectory || {} - // The backend writes a per-run `error` field into the run cache when - // the workflow raises (see mcp_svc._run_execution's except branch). - // Surface it so ChatMessage can render the actual cause under the - // "Run failed." headline instead of leaving it blank. + // The run cache carries the workflow's own exception text; without this + // ChatMessage renders "Run failed." with no cause. if (data.error) errorMessage.value = data.error } - // True once this composable has moved on from `id`, which reset() signals by - // clearing runId. "New chat" resets mid-run, so a GET issued for the run the - // user just walked away from can still be in flight; letting it land would - // strand the view on a RUNNING status nothing polls any more. + // reset() signals "moved on" by clearing runId, so a GET issued for the + // abandoned run must not land on the state that replaced it. function _superseded(id) { return runId.value !== id } - // Pull one status snapshot into the reactive state. Returns false once - // polling should stop, either because the run reached a terminal state or - // because the endpoint will not answer for this run again. + // Returns false once polling should stop: terminal state, or superseded. async function _pollOnce(id) { try { const res = await $api.get('/plugin/mcp/status', { params: { run_id: id } }) if (_superseded(id)) return false + if (!_isSnapshot(res.data)) { + status.value = 'FAILED' + errorMessage.value = SESSION_EXPIRED + return false + } consecutivePollErrors = 0 _applySnapshot(res.data) return status.value !== 'FINISHED' && status.value !== 'FAILED' } catch (err) { if (_superseded(id)) return false - // 404 means the run left the live cache: the server restarted, or the - // run aged out of the LRU bound. Neither resolves by asking again. + // 404 means the run left the live cache: server restart, or LRU + // eviction. Neither resolves by asking again. if (err?.response?.status === 404) { status.value = 'FAILED' errorMessage.value = @@ -135,27 +145,33 @@ export function useMcpRun($api) { } consecutivePollErrors += 1 if (consecutivePollErrors < MAX_CONSECUTIVE_POLL_ERRORS) return true + // Losing the poll says nothing about the run, which is still going. status.value = 'FAILED' - errorMessage.value = 'Polling failed.' + errorMessage.value = + 'Lost contact with the server while this run was in progress. ' + + 'Check the History tab for its result.' return false } } function _beginPolling(id) { _clearTimer() - // Scoped per polling session so a slow status GET cannot stack up behind - // itself when the server takes longer than one interval to answer. + // Per polling session, so a slow GET cannot stack up behind itself. let inFlight = false - pollTimer = setInterval(async () => { + const timer = setInterval(async () => { if (inFlight) return inFlight = true try { const stillLive = await _pollOnce(id) - if (!stillLive) _clearTimer() + // Only ever clear our own timer: clearInterval cannot cancel a + // callback already suspended, so a superseded run can land here + // after a newer one installed its poller. + if (!stillLive && pollTimer === timer) _clearTimer() } finally { inFlight = false } }, POLL_INTERVAL_MS) + pollTimer = timer } /** Stop polling without touching run state; the server run keeps going. */