mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 16:02:59 +00:00
fix(sessions): scope entity-tab effects to the mutated item, tab and workspace
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
9deb95a63e
commit
ba1ae123fe
@@ -687,10 +687,12 @@ export function pendingUserActionDetail(
|
||||
// the sessions page) react to mutating tools — refreshing previews — without
|
||||
// the tool layer knowing about the UI. Single slot; the consumer filters by name
|
||||
// and reads the tool args (e.g. the mutated item's `path`) to scope its refresh.
|
||||
let toolCompletionListener: ((toolName: string, args: any) => void) | undefined
|
||||
let toolCompletionListener:
|
||||
| ((toolName: string, args: any, workspace: string) => void)
|
||||
| undefined
|
||||
|
||||
export function setToolCompletionListener(
|
||||
fn: ((toolName: string, args: any) => void) | undefined
|
||||
fn: ((toolName: string, args: any, workspace: string) => void) | undefined
|
||||
): void {
|
||||
toolCompletionListener = fn
|
||||
}
|
||||
@@ -719,7 +721,9 @@ async function callTool<T>({
|
||||
)
|
||||
}
|
||||
const result = await tool.fn({ args, workspace, helpers, toolCallbacks, toolId })
|
||||
toolCompletionListener?.(functionName, args)
|
||||
// The workspace the tool acted on, so a consumer can tell a mutation in this
|
||||
// session's workspace from the same path in another one.
|
||||
toolCompletionListener?.(functionName, args, workspace)
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
@@ -118,11 +118,11 @@
|
||||
if (slot.kind === 'editor') return
|
||||
// An entity editor holds a live UserDraft handle on the cell the chat's
|
||||
// write seeds, so a write needs nothing from here — but the tools that drop
|
||||
// that cell go behind it: it has to be re-read from the server, or left
|
||||
// when the item it edits no longer exists.
|
||||
// that cell go behind it, and it has to be re-read from the server.
|
||||
// Deletion is not handled here: it belongs to the tab whose item is gone,
|
||||
// mounted or not, so the page re-points that tab by id instead.
|
||||
if (slot.kind === 'entity') {
|
||||
if (opts?.entity === 'close') backToList?.()
|
||||
else if (opts?.entity === 'refresh') entityNonce++
|
||||
if (opts?.entity === 'refresh') entityNonce++
|
||||
return
|
||||
}
|
||||
try {
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { toolReloadEffect, tabsToReload, strongerEntityEffect } from './previewReload'
|
||||
import {
|
||||
toolReloadEffect,
|
||||
tabsToReload,
|
||||
strongerEntityEffect,
|
||||
entityEffectForTab
|
||||
} from './previewReload'
|
||||
import type { SessionPreviewTab } from './sessionState.svelte'
|
||||
|
||||
describe('toolReloadEffect', () => {
|
||||
@@ -40,6 +45,14 @@ describe('toolReloadEffect', () => {
|
||||
expect(toolReloadEffect('delete_workspace_item', { type: 'resource' }).entity).toBe('close')
|
||||
})
|
||||
|
||||
it('carries the mutated item path, so an editor on another item is left alone', () => {
|
||||
expect(
|
||||
toolReloadEffect('delete_workspace_item', { type: 'resource', path: 'u/me/a' }).path
|
||||
).toBe('u/me/a')
|
||||
// No path in the args: nothing to scope by, so it reaches every editor on the page.
|
||||
expect(toolReloadEffect('deploy_workspace_item', { type: 'resource' }).path).toBeUndefined()
|
||||
})
|
||||
|
||||
it('takes the strongest effect across a debounced round', () => {
|
||||
expect(strongerEntityEffect('none', 'refresh')).toBe('refresh')
|
||||
expect(strongerEntityEffect('refresh', 'close')).toBe('close')
|
||||
@@ -72,6 +85,44 @@ describe('toolReloadEffect', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('entityEffectForTab', () => {
|
||||
const del = (path: string, workspace = 'ws1') => ({
|
||||
pages: ['/resources'],
|
||||
effect: 'close' as const,
|
||||
path,
|
||||
workspace
|
||||
})
|
||||
const tab = { listPage: '/resources', path: 'u/me/a', workspace: 'ws1' }
|
||||
|
||||
it('applies a mutation to the editor on that item', () => {
|
||||
expect(entityEffectForTab([del('u/me/a')], tab)).toBe('close')
|
||||
})
|
||||
|
||||
// The bug this guards: deleting one resource must not shut every open resource
|
||||
// editor, nor the same path in a session acting on another workspace.
|
||||
it('leaves editors on another item, page, or workspace alone', () => {
|
||||
expect(entityEffectForTab([del('u/me/b')], tab)).toBe('none')
|
||||
expect(entityEffectForTab([del('u/me/a', 'ws2')], tab)).toBe('none')
|
||||
expect(entityEffectForTab([{ ...del('u/me/a'), pages: ['/variables'] }], tab)).toBe('none')
|
||||
})
|
||||
|
||||
it('reaches every editor on the page when the tool named no item', () => {
|
||||
expect(entityEffectForTab([{ ...del('u/me/a'), path: undefined }], tab)).toBe('close')
|
||||
})
|
||||
|
||||
it('takes the strongest of the mutations that reach it, not of the whole round', () => {
|
||||
const refreshOther = {
|
||||
pages: ['/resources'],
|
||||
effect: 'refresh' as const,
|
||||
path: 'u/me/b',
|
||||
workspace: 'ws1'
|
||||
}
|
||||
const refreshMine = { ...refreshOther, path: 'u/me/a' }
|
||||
expect(entityEffectForTab([refreshMine, del('u/me/b')], tab)).toBe('refresh')
|
||||
expect(entityEffectForTab([refreshMine, del('u/me/a')], tab)).toBe('close')
|
||||
})
|
||||
})
|
||||
|
||||
describe('tabsToReload', () => {
|
||||
const scheduleTab: SessionPreviewTab = { id: 's', url: '/schedules', loc: '/schedules' }
|
||||
const resourceTab: SessionPreviewTab = { id: 'r', url: '/resources', loc: '/resources' }
|
||||
|
||||
@@ -22,7 +22,14 @@ import { stripBase, TRIGGER_PAGES, type TriggerKind } from './previewPaths'
|
||||
* altogether. `none` is what makes the live-editing case live. */
|
||||
export type EntityToolEffect = 'none' | 'refresh' | 'close'
|
||||
|
||||
export type ToolReloadEffect = { pages: string[]; entity: EntityToolEffect }
|
||||
export type ToolReloadEffect = {
|
||||
pages: string[]
|
||||
entity: EntityToolEffect
|
||||
/** The item the tool mutated, when its args name one. A hosted editor on
|
||||
* another path is not affected by it — unlike a list page, which shows every
|
||||
* row and so reloads for any mutation on it. */
|
||||
path?: string
|
||||
}
|
||||
const NO_RELOAD: ToolReloadEffect = { pages: [], entity: 'none' }
|
||||
|
||||
export function toolReloadEffect(name: string, args: any): ToolReloadEffect {
|
||||
@@ -46,15 +53,46 @@ export function toolReloadEffect(name: string, args: any): ToolReloadEffect {
|
||||
case 'discard_local_draft':
|
||||
case 'deploy_workspace_item':
|
||||
case 'rebase_draft':
|
||||
return { pages: pagesForItemType(args?.type, args), entity: 'refresh' }
|
||||
return { pages: pagesForItemType(args?.type, args), entity: 'refresh', path: itemPath(args) }
|
||||
case 'delete_workspace_item':
|
||||
return { pages: pagesForItemType(args?.type, args), entity: 'close' }
|
||||
return { pages: pagesForItemType(args?.type, args), entity: 'close', path: itemPath(args) }
|
||||
default:
|
||||
return NO_RELOAD
|
||||
}
|
||||
}
|
||||
|
||||
/** The stronger of two effects, for a debounced round that saw several tools:
|
||||
function itemPath(args: any): string | undefined {
|
||||
return typeof args?.path === 'string' && args.path ? args.path : undefined
|
||||
}
|
||||
|
||||
/** One item mutation from a chat round, as the preview needs to read it back. */
|
||||
export type EntityMutation = {
|
||||
pages: string[]
|
||||
effect: EntityToolEffect
|
||||
path?: string
|
||||
/** The workspace the tool acted on — a session on a fork must not be moved by
|
||||
* a mutation to the same path in its parent. */
|
||||
workspace: string
|
||||
}
|
||||
|
||||
/** What a hosted entity editor showing `tab` must do about a round's mutations.
|
||||
* A mutation reaches it only in its own workspace, on its own list page, and —
|
||||
* when the tool named one — on its own path. */
|
||||
export function entityEffectForTab(
|
||||
mutations: readonly EntityMutation[],
|
||||
tab: { listPage: string; path: string; workspace: string }
|
||||
): EntityToolEffect {
|
||||
let effect: EntityToolEffect = 'none'
|
||||
for (const m of mutations) {
|
||||
if (m.workspace !== tab.workspace) continue
|
||||
if (!m.pages.includes(tab.listPage)) continue
|
||||
if (m.path !== undefined && m.path !== tab.path) continue
|
||||
effect = strongerEntityEffect(effect, m.effect)
|
||||
}
|
||||
return 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 {
|
||||
if (a === 'close' || b === 'close') return 'close'
|
||||
|
||||
@@ -301,6 +301,18 @@ export class SessionPreviewTabs {
|
||||
if ((commandUnchanged && drifted) || fragmentOnly) this.pulseReload(tab.id)
|
||||
}
|
||||
|
||||
/** Re-point one tab by id. `navigate` moves whichever tab is active, which is
|
||||
* what a user's own pick means; this is for a change that belongs to a
|
||||
* particular tab whether or not the user is looking at it — the item a hosted
|
||||
* entity editor was showing has been deleted out from under it. Works on a
|
||||
* tab with no mounted host, which would otherwise never hear about it. */
|
||||
retargetTabTo(id: string, url: string): void {
|
||||
const tab = this.#tabs.find((t) => t.id === id)
|
||||
if (!tab) return
|
||||
this.#retarget(tab, url)
|
||||
this.#flush()
|
||||
}
|
||||
|
||||
// Force the host to reload the iframe. A navigation onto the tab's exact current URL
|
||||
// changes nothing, so URL-driven behavior — a `#<path>` opening a drawer the user has
|
||||
// since closed — would never re-fire.
|
||||
|
||||
@@ -389,7 +389,12 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
// point those at a page the user cannot see.
|
||||
const tab = owner?.displayedTab
|
||||
if (!tab) return undefined
|
||||
if (resolvePreviewTab(tab.url).kind !== 'iframe') return undefined
|
||||
// Entity editors are hosted in process but, unlike the item editors, they
|
||||
// register no live-editor draft — so this is the only thing that tells the
|
||||
// model which schedule/resource/variable is on screen, and "change this
|
||||
// one" would otherwise resolve against nothing.
|
||||
const kind = resolvePreviewTab(tab.url).kind
|
||||
if (kind !== 'iframe' && kind !== 'entity') return undefined
|
||||
return previewLocationContext(whereIs(tab))
|
||||
}
|
||||
// Pre-flight: materialise the (still-transient) session, then commit
|
||||
|
||||
@@ -38,7 +38,7 @@
|
||||
} from '$lib/components/sessions/sessionState.svelte'
|
||||
import { withWorkspaceParam } from '$lib/components/sessions/sessionMode.svelte'
|
||||
import { enterSessionMode } from '$lib/components/sessions/sessionSwitch.svelte'
|
||||
import type { SessionPreviewTabs } from '$lib/components/sessions/sessionPreviewTabs.svelte'
|
||||
import { whereIs, type SessionPreviewTabs } from '$lib/components/sessions/sessionPreviewTabs.svelte'
|
||||
import { userStore, userWorkspaces, usersWorkspaceStore, workspaceStore } from '$lib/stores'
|
||||
import {
|
||||
getOrCreateRuntime,
|
||||
@@ -61,16 +61,19 @@
|
||||
matchPreviewPage,
|
||||
pageKey,
|
||||
parseArtifactRoute,
|
||||
entityListPage,
|
||||
pageHref,
|
||||
parseEntityEditorRoute,
|
||||
parsePreviewItemRoute,
|
||||
previewLocationLabel,
|
||||
stripBase,
|
||||
type PreviewTarget
|
||||
} from '$lib/components/sessions/previewRouter'
|
||||
import {
|
||||
toolReloadEffect,
|
||||
tabsToReload,
|
||||
strongerEntityEffect,
|
||||
type EntityToolEffect
|
||||
entityEffectForTab,
|
||||
type EntityMutation
|
||||
} from '$lib/components/sessions/previewReload'
|
||||
import {
|
||||
leafKeyFor,
|
||||
@@ -562,45 +565,64 @@
|
||||
// Base-stripped list-page paths (e.g. `/schedules`) a chat round touched since
|
||||
// the last flush — see toolReloadEffect for how tools map to pages.
|
||||
let pendingPages = new Set<string>()
|
||||
// What those tools ask of a hosted entity editor on one of those pages —
|
||||
// carried alongside the paths because the debounce below loses which tool
|
||||
// contributed which page.
|
||||
let pendingEntity: EntityToolEffect = 'none'
|
||||
// The same round's mutations, kept whole rather than folded into one verdict:
|
||||
// a hosted entity editor is only affected by a mutation to its own item, in
|
||||
// its own workspace, so the path and workspace have to survive the debounce.
|
||||
let pendingMutations: EntityMutation[] = []
|
||||
|
||||
// Reload the mounted list-page tabs a chat round changed, across all warm
|
||||
// sessions (a hidden preview would otherwise show pre-mutation content on
|
||||
// return). tabsToReload picks only the tabs whose page is in `pages`.
|
||||
function reloadTabs(pages: Set<string>, entity: EntityToolEffect) {
|
||||
function reloadTabs(pages: Set<string>, mutations: EntityMutation[]) {
|
||||
for (const s of warmSessions) {
|
||||
const owner = getRuntime(s.id)?.previewTabs
|
||||
if (!owner) continue
|
||||
const workspace = getEffectiveWorkspaceId(s)
|
||||
for (const tab of tabsToReload(owner.tabs, pages)) {
|
||||
const key = tabKey(s.id, tab.id)
|
||||
if (mountedTabKeys.has(key)) tabHosts[key]?.reload({ entity })
|
||||
// A list tab shows every row, so any mutation on its page is its
|
||||
// business. A hosted entity tab shows one item, and is told apart here.
|
||||
const entity = parseEntityEditorRoute(whereIs(tab))
|
||||
if (!entity) {
|
||||
if (mountedTabKeys.has(key)) tabHosts[key]?.reload()
|
||||
continue
|
||||
}
|
||||
const listPage = stripBase(whereIs(tab))
|
||||
const effect = workspace
|
||||
? entityEffectForTab(mutations, { listPage, path: entity.path, workspace })
|
||||
: 'none'
|
||||
// Deletion is re-pointed on the tab model, so a tab whose host is not
|
||||
// mounted is not left sitting on an item that no longer exists.
|
||||
if (effect === 'close') {
|
||||
const page = entityListPage(entity.entityKind)
|
||||
if (page) owner.retargetTabTo(tab.id, pageHref(page.path))
|
||||
} else if (effect === 'refresh' && mountedTabKeys.has(key)) {
|
||||
tabHosts[key]?.reload({ entity: 'refresh' })
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
function flushReload() {
|
||||
const pages = pendingPages
|
||||
const entity = pendingEntity
|
||||
const mutations = pendingMutations
|
||||
pendingPages = new Set()
|
||||
pendingEntity = 'none'
|
||||
reloadTabs(pages, entity)
|
||||
pendingMutations = []
|
||||
reloadTabs(pages, mutations)
|
||||
}
|
||||
$effect(() => {
|
||||
// Debounced so a burst of writes (the AI editing several files) reloads once.
|
||||
setToolCompletionListener((name, args) => {
|
||||
const { pages, entity } = toolReloadEffect(name, args)
|
||||
setToolCompletionListener((name, args, workspace) => {
|
||||
const { pages, entity, path } = toolReloadEffect(name, args)
|
||||
if (pages.length === 0) return
|
||||
for (const p of pages) pendingPages.add(p)
|
||||
pendingEntity = strongerEntityEffect(pendingEntity, entity)
|
||||
if (entity !== 'none') pendingMutations.push({ pages, effect: entity, path, workspace })
|
||||
clearTimeout(reloadHandle)
|
||||
reloadHandle = setTimeout(flushReload, 500)
|
||||
})
|
||||
return () => {
|
||||
clearTimeout(reloadHandle)
|
||||
pendingPages = new Set()
|
||||
pendingEntity = 'none'
|
||||
pendingMutations = []
|
||||
setToolCompletionListener(undefined)
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user