diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index 6d6794b56c..8bb263a77b 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -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({ ) } 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 } diff --git a/frontend/src/lib/components/sessions/PreviewTabHost.svelte b/frontend/src/lib/components/sessions/PreviewTabHost.svelte index c131369a5e..a029b34caa 100644 --- a/frontend/src/lib/components/sessions/PreviewTabHost.svelte +++ b/frontend/src/lib/components/sessions/PreviewTabHost.svelte @@ -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 { diff --git a/frontend/src/lib/components/sessions/previewReload.test.ts b/frontend/src/lib/components/sessions/previewReload.test.ts index 6a7d6bff88..5e24e12f34 100644 --- a/frontend/src/lib/components/sessions/previewReload.test.ts +++ b/frontend/src/lib/components/sessions/previewReload.test.ts @@ -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' } diff --git a/frontend/src/lib/components/sessions/previewReload.ts b/frontend/src/lib/components/sessions/previewReload.ts index 3c4a9b05d7..1847f8d6cb 100644 --- a/frontend/src/lib/components/sessions/previewReload.ts +++ b/frontend/src/lib/components/sessions/previewReload.ts @@ -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' diff --git a/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts b/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts index 23cf60c421..0f454c777c 100644 --- a/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts @@ -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 `#` opening a drawer the user has // since closed — would never re-fire. diff --git a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts index c49a464962..c866f2ce6b 100644 --- a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts @@ -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 diff --git a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte index 81c1b63464..231ab4ca9e 100644 --- a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte @@ -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() - // 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, entity: EntityToolEffect) { + function reloadTabs(pages: Set, 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) } })