mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 08:02:47 +00:00
fix(sessions): leave a hosted editor whose draft-only item was discarded
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4
This commit is contained in:
co-authored by
Claude Opus 5
parent
82cf519606
commit
98e30f0878
@@ -6135,6 +6135,25 @@ function createOpenVariableAction(path: string): ToolDisplayAction {
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether a deployed item exists under a draft, for the kinds whose editor a
|
||||
* session hosts. Anything else answers true — the caller only uses this to decide
|
||||
* whether discarding leaves nothing behind, and no other kind is hosted. */
|
||||
async function hasDeployedItem(
|
||||
workspace: string,
|
||||
type: WorkspaceItemType,
|
||||
path: string
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
if (type === 'resource') return await ResourceService.existsResource({ workspace, path })
|
||||
if (type === 'variable') return await VariableService.existsVariable({ workspace, path })
|
||||
if (type === 'schedule') return await ScheduleService.existsSchedule({ workspace, path })
|
||||
} catch {
|
||||
// The probe is an optimisation over "assume it survived"; a failed one must
|
||||
// not fail the discard, and treating it as deployed keeps today's behavior.
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
async function discardLocalDraft(
|
||||
args: { type: WorkspaceItemType; path: string; trigger_kind?: TriggerKind },
|
||||
ctx: WriteDraftCtx
|
||||
@@ -6151,16 +6170,21 @@ async function discardLocalDraft(
|
||||
throw new Error(`No draft found for ${type} "${path}".`)
|
||||
}
|
||||
|
||||
// Before the delete, while both sides are still knowable: a draft with nothing
|
||||
// deployed under it IS the item, so discarding it removes the item rather than
|
||||
// reverting it — and anything showing that item has to stop.
|
||||
const discardedKind = itemKindFor(type, triggerKind)
|
||||
const storagePath = getGlobalDraftStoragePath(workspace, type, path, triggerKind)
|
||||
if (discardedKind && !(await hasDeployedItem(workspace, type, path))) {
|
||||
UserDraft.recordDraftOnlyDiscard(discardedKind, storagePath, { workspace })
|
||||
}
|
||||
|
||||
await deleteGlobalDraft(workspace, type, path, triggerKind)
|
||||
|
||||
// The chat's touch on the item is undone — drop it from the mask so a
|
||||
// pre-existing deployed item doesn't keep reading as this chat's edit.
|
||||
const discardedKind = itemKindFor(type, triggerKind)
|
||||
if (discardedKind) {
|
||||
toolCallbacks.onItemDiscarded?.(
|
||||
discardedKind,
|
||||
getGlobalDraftStoragePath(workspace, type, path, triggerKind)
|
||||
)
|
||||
toolCallbacks.onItemDiscarded?.(discardedKind, storagePath)
|
||||
}
|
||||
|
||||
toolCallbacks.setToolStatus(toolId, {
|
||||
|
||||
@@ -4,7 +4,8 @@ import {
|
||||
tabsToReload,
|
||||
strongerEntityEffect,
|
||||
entityEffectForTab,
|
||||
effectForWrite
|
||||
effectForWrite,
|
||||
effectForDiscard
|
||||
} from './previewReload'
|
||||
import type { SessionPreviewTab } from './sessionState.svelte'
|
||||
|
||||
@@ -101,6 +102,24 @@ describe('effectForWrite', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('effectForDiscard', () => {
|
||||
// Discarding reverts an item to what is deployed — unless nothing is, in which
|
||||
// case the draft WAS the item and the editor is left showing something that no
|
||||
// longer exists, remounting into a load that cannot resolve.
|
||||
it('closes a hosted editor whose item the discard removed outright', () => {
|
||||
expect(effectForDiscard('refresh', false)).toBe('close')
|
||||
})
|
||||
|
||||
it('keeps a deployed item on screen, reverted rather than closed', () => {
|
||||
expect(effectForDiscard('refresh', true)).toBe('refresh')
|
||||
})
|
||||
|
||||
it('leaves the other effects alone', () => {
|
||||
expect(effectForDiscard('none', false)).toBe('none')
|
||||
expect(effectForDiscard('close', true)).toBe('close')
|
||||
})
|
||||
})
|
||||
|
||||
describe('entityEffectForTab', () => {
|
||||
const del = (path: string, workspace = 'ws1') => ({
|
||||
pages: ['/resources'],
|
||||
|
||||
@@ -109,6 +109,17 @@ export function effectForWrite(
|
||||
return effect === 'none' && !seedReachedEditor ? 'refresh' : effect
|
||||
}
|
||||
|
||||
/** What a discard asks of a hosted editor. Discarding a draft that had a deployed
|
||||
* item under it reverts the editor to that item, which a refresh shows — but a
|
||||
* draft-only item has nothing to revert to, so the discard removed it and the
|
||||
* editor has to leave rather than remount into a load that cannot resolve. */
|
||||
export function effectForDiscard(
|
||||
effect: EntityToolEffect,
|
||||
itemSurvives: boolean
|
||||
): EntityToolEffect {
|
||||
return effect === 'refresh' && !itemSurvives ? 'close' : effect
|
||||
}
|
||||
|
||||
/** The stronger of two effects, for a tab several of a round's mutations reach:
|
||||
* a delete outranks a refresh, which outranks nothing. */
|
||||
export function strongerEntityEffect(a: EntityToolEffect, b: EntityToolEffect): EntityToolEffect {
|
||||
|
||||
@@ -127,6 +127,8 @@ const entries = new Map<string, DraftEntry>()
|
||||
// Cells whose last `seed` found no live entry (see `takeSeedMiss`). Bounded by
|
||||
// the seeds that missed and not yet been read back, and each entry is one key.
|
||||
const seedMisses = new Set<string>()
|
||||
// Cells whose last discard removed the item itself (see `takeDraftOnlyDiscard`).
|
||||
const draftOnlyDiscards = new Set<string>()
|
||||
const liveEditorDrafts = new Map<string, LiveEditorDraft>()
|
||||
/**
|
||||
* Map keys whose entry should start `syncSuspended` on acquire. Lets
|
||||
@@ -415,6 +417,26 @@ export const UserDraft = {
|
||||
entry.state.val = snapshotDraftValue(value)
|
||||
},
|
||||
|
||||
/**
|
||||
* Record that the draft just discarded for this cell was the item's only stored
|
||||
* form — nothing deployed underneath, so the item is now gone rather than
|
||||
* reverted. Read once by a consumer showing that item, which has to stop
|
||||
* showing it. Set at the discard, where the deployed side is known.
|
||||
*/
|
||||
recordDraftOnlyDiscard(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void {
|
||||
draftOnlyDiscards.add(mapKey(resolveWorkspace(opts), itemKind, path))
|
||||
},
|
||||
|
||||
/** Whether the last discard for this cell removed the item outright (see
|
||||
* {@link recordDraftOnlyDiscard}), clearing the record. */
|
||||
takeDraftOnlyDiscard(
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string,
|
||||
opts?: UserDraftOptions
|
||||
): boolean {
|
||||
return draftOnlyDiscards.delete(mapKey(resolveWorkspace(opts), itemKind, path))
|
||||
},
|
||||
|
||||
/**
|
||||
* Whether the last {@link seed} for this cell found no live entry, clearing the
|
||||
* record. An editor acquires its cell only once its first load resolves, so a
|
||||
|
||||
@@ -77,6 +77,7 @@
|
||||
tabsToReload,
|
||||
entityEffectForTab,
|
||||
effectForWrite,
|
||||
effectForDiscard,
|
||||
type EntityMutation
|
||||
} from '$lib/components/sessions/previewReload'
|
||||
import {
|
||||
@@ -626,7 +627,11 @@
|
||||
// the editor can have acquired the cell during the write's round-trip.
|
||||
const kind = path ? entityKindForPage(pages[0]) : undefined
|
||||
const seedReachedEditor = !kind || !path || !UserDraft.takeSeedMiss(kind, path, { workspace })
|
||||
const effect = effectForWrite(entity, seedReachedEditor)
|
||||
// A discard of a draft-only item removed the item itself, so its editor has
|
||||
// nothing left to re-read and must leave instead.
|
||||
const itemSurvives =
|
||||
!kind || !path || !UserDraft.takeDraftOnlyDiscard(kind, path, { workspace })
|
||||
const effect = effectForDiscard(effectForWrite(entity, seedReachedEditor), itemSurvives)
|
||||
if (effect !== 'none') pendingMutations.push({ pages, effect, path, workspace })
|
||||
clearTimeout(reloadHandle)
|
||||
reloadHandle = setTimeout(flushReload, 500)
|
||||
|
||||
Reference in New Issue
Block a user