diff --git a/frontend/src/lib/components/ScriptEditor.svelte b/frontend/src/lib/components/ScriptEditor.svelte index 4fc3e4cfaf..1ca54e619d 100644 --- a/frontend/src/lib/components/ScriptEditor.svelte +++ b/frontend/src/lib/components/ScriptEditor.svelte @@ -1213,14 +1213,15 @@ // false and the chat survives; the fresh onMount then sees mode is still // SCRIPT and skips its clearing saveAndClear. let leaveOnDestroy = $state(false) - beforeNavigate(({ to }) => { - // Recompute on every navigation (both branches) so a stale decision from - // an earlier same-script navigation never lingers into a later - // cross-script one. + beforeNavigate(({ from, to }) => { + // Recompute on every navigation (both branches) so a stale decision never + // lingers. Decided from the route pathnames so a new/draft script (whose + // scriptEditorOptions.path is undefined) still clears when navigating away. leaveOnDestroy = !navStaysInEditor( + from?.url.pathname ?? '', to?.url.pathname ?? '', - '/scripts/edit/', - aiChatManager.scriptEditorOptions?.path + '/scripts/add', + '/scripts/edit/' ) }) diff --git a/frontend/src/lib/components/copilot/chat/editorNav.test.ts b/frontend/src/lib/components/copilot/chat/editorNav.test.ts index 9e5719dcb8..3e533e50cf 100644 --- a/frontend/src/lib/components/copilot/chat/editorNav.test.ts +++ b/frontend/src/lib/components/copilot/chat/editorNav.test.ts @@ -2,37 +2,73 @@ import { describe, it, expect } from 'vitest' import { navStaysInEditor } from './editorNav' describe('navStaysInEditor', () => { - const FLOW = '/flows/edit/' + const ADD = '/flows/add' + const EDIT = '/flows/edit/' - it('preserves on the add → edit promotion (no current path yet)', () => { - // On /flows/add the entity has no path; the first save navigates to - // /flows/edit/{path}. Both undefined and '' must count as "promotion". - expect(navStaysInEditor('/flows/edit/u/admin/foo', FLOW, undefined)).toBe(true) - expect(navStaysInEditor('/flows/edit/u/admin/foo', FLOW, '')).toBe(true) + it('preserves on the add → edit promotion', () => { + expect(navStaysInEditor('/flows/add', '/flows/edit/u/admin/foo', ADD, EDIT)).toBe(true) }) - it('preserves when staying on the same entity', () => { - expect(navStaysInEditor('/flows/edit/u/admin/foo', FLOW, 'u/admin/foo')).toBe(true) + it('preserves when staying on the same entity (e.g. a query change)', () => { + expect(navStaysInEditor('/flows/edit/u/admin/foo', '/flows/edit/u/admin/foo', ADD, EDIT)).toBe( + true + ) }) it('clears when navigating to a different entity', () => { - expect(navStaysInEditor('/flows/edit/u/admin/bar', FLOW, 'u/admin/foo')).toBe(false) + expect(navStaysInEditor('/flows/edit/u/admin/foo', '/flows/edit/u/admin/bar', ADD, EDIT)).toBe( + false + ) + }) + + it('clears cross-entity even when editor state would be undefined (the bug)', () => { + // The old signature used the open entity's path as the promotion signal, + // which is undefined for a new/draft flow — making this wrongly preserve. + // Deciding from `from` (the real current route) fixes it: a draft flow's + // edit route is still a concrete `/flows/edit/{path}`. + expect( + navStaysInEditor('/flows/edit/u/admin/draftflow', '/flows/edit/u/admin/other', ADD, EDIT) + ).toBe(false) }) it('clears when leaving the editor entirely', () => { - expect(navStaysInEditor('/', FLOW, 'u/admin/foo')).toBe(false) - expect(navStaysInEditor('/scripts/edit/u/admin/foo', FLOW, 'u/admin/foo')).toBe(false) - expect(navStaysInEditor('/flows/get/u/admin/foo', FLOW, 'u/admin/foo')).toBe(false) + expect(navStaysInEditor('/flows/edit/u/admin/foo', '/', ADD, EDIT)).toBe(false) + expect( + navStaysInEditor('/flows/edit/u/admin/foo', '/scripts/edit/u/admin/foo', ADD, EDIT) + ).toBe(false) + expect(navStaysInEditor('/flows/edit/u/admin/foo', '/flows/get/u/admin/foo', ADD, EDIT)).toBe( + false + ) }) it('does not treat a prefixed-but-different route as the editor', () => { - // '/flows/edit-history/...' must not be mistaken for '/flows/edit/...' - expect(navStaysInEditor('/flows/edit-history/foo', FLOW, undefined)).toBe(false) + expect(navStaysInEditor('/flows/add', '/flows/edit-history/foo', ADD, EDIT)).toBe(false) }) - it('works for the raw-app and script prefixes too', () => { - expect(navStaysInEditor('/apps_raw/edit/u/admin/a', '/apps_raw/edit/', '')).toBe(true) - expect(navStaysInEditor('/apps_raw/edit/u/admin/b', '/apps_raw/edit/', 'u/admin/a')).toBe(false) - expect(navStaysInEditor('/scripts/edit/u/admin/s', '/scripts/edit/', 'u/admin/s')).toBe(true) + it('works for the raw-app and script add/edit routes too', () => { + expect( + navStaysInEditor( + '/apps_raw/add', + '/apps_raw/edit/u/admin/a', + '/apps_raw/add', + '/apps_raw/edit/' + ) + ).toBe(true) + expect( + navStaysInEditor( + '/apps_raw/edit/u/admin/a', + '/apps_raw/edit/u/admin/b', + '/apps_raw/add', + '/apps_raw/edit/' + ) + ).toBe(false) + expect( + navStaysInEditor( + '/scripts/edit/u/admin/s', + '/scripts/edit/u/admin/s', + '/scripts/add', + '/scripts/edit/' + ) + ).toBe(true) }) }) diff --git a/frontend/src/lib/components/copilot/chat/editorNav.ts b/frontend/src/lib/components/copilot/chat/editorNav.ts index 1d2c938766..aa3792567e 100644 --- a/frontend/src/lib/components/copilot/chat/editorNav.ts +++ b/frontend/src/lib/components/copilot/chat/editorNav.ts @@ -1,27 +1,38 @@ /** - * Whether a navigation destination keeps the user within the same editor - * entity, so the AI chat about that entity should be preserved across the - * resulting unmount/remount instead of cleared. + * Whether a navigation from `fromPathname` to `toPathname` stays within the + * same editor entity, so the AI chat about that entity should be preserved + * across the resulting unmount/remount instead of cleared. * - * Used by FlowEditor / ScriptEditor / RawAppEditor to decide, in a - * `beforeNavigate` guard, whether leaving for `destPathname` is a real leave - * (clear the chat) or an intra-editor transition (keep it): + * Preserves when: + * - the destination is the SAME entity currently open + * (`{editPrefix}A` → `{editPrefix}A`, e.g. a `?selected=…` query change), or + * - the `{addPathname}` → `{editPrefix}{path}` first-save promotion. * - * - `editPrefix` is the editor's edit-route prefix, e.g. `'/flows/edit/'`. - * - `currentPath` is the entity currently open. It is `undefined`/`''` on the - * add page, which is exactly the `add → edit/{path}` first-save promotion — - * that case must preserve, hence the `!currentPath` branch. + * Clears for a different entity, or any destination outside this editor. * - * Note this only covers *navigations*. Non-navigation remounts (e.g. an edit - * page wiping then reloading its data) fire no `beforeNavigate`, so callers - * default to preserving in that case. + * This decides purely from the route pathnames (both reliably available from a + * `beforeNavigate` callback's `from`/`to`) rather than from editor state such as + * `flowOptions.path`, which is undefined for a new or draft-only entity — using + * that as the promotion signal made cross-entity navigation wrongly preserve. + * + * Used by FlowEditor / ScriptEditor / RawAppEditor. Non-navigation remounts + * (e.g. an edit page wiping then reloading its data) fire no `beforeNavigate`, + * so callers default to preserving in that case. */ export function navStaysInEditor( - destPathname: string, - editPrefix: string, - currentPath: string | undefined + fromPathname: string, + toPathname: string, + addPathname: string, + editPrefix: string ): boolean { - if (!destPathname.startsWith(editPrefix)) return false - const destPath = destPathname.slice(editPrefix.length) - return !currentPath || destPath === currentPath + // Leaving this editor entirely (home, a different section, the entity's + // non-edit pages, …). + if (!toPathname.startsWith(editPrefix)) return false + // `{addPathname}` → `{editPrefix}{path}`: the first-save promotion. + if (fromPathname === addPathname) return true + // Same entity (e.g. a `?selected=…` query change on the same edit route). + if (fromPathname.startsWith(editPrefix)) { + return fromPathname.slice(editPrefix.length) === toPathname.slice(editPrefix.length) + } + return false } diff --git a/frontend/src/lib/components/flows/FlowEditor.svelte b/frontend/src/lib/components/flows/FlowEditor.svelte index f123380510..3465538b50 100644 --- a/frontend/src/lib/components/flows/FlowEditor.svelte +++ b/frontend/src/lib/components/flows/FlowEditor.svelte @@ -147,15 +147,15 @@ // so leaveOnDestroy stays false and the chat survives; the fresh onMount then // sees mode is still FLOW and skips its clearing saveAndClear. let leaveOnDestroy = $state(false) - beforeNavigate(({ to }) => { - // Recompute on every navigation (both branches) so a stale decision from - // an earlier same-flow navigation never lingers into a later cross-flow - // one. The add page has no flowOptions.path yet, which navStaysInEditor - // treats as the add→edit promotion (preserve). + beforeNavigate(({ from, to }) => { + // Recompute on every navigation (both branches) so a stale decision never + // lingers. Decided from the route pathnames so a new/draft flow (whose + // flowOptions.path is undefined) still clears when navigating cross-flow. leaveOnDestroy = !navStaysInEditor( + from?.url.pathname ?? '', to?.url.pathname ?? '', - '/flows/edit/', - aiChatManager.flowOptions?.path + '/flows/add', + '/flows/edit/' ) }) diff --git a/frontend/src/lib/components/raw_apps/RawAppEditor.svelte b/frontend/src/lib/components/raw_apps/RawAppEditor.svelte index 2ce5447519..8534ba4d06 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditor.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditor.svelte @@ -628,10 +628,16 @@ // beforeNavigate, so leaveOnDestroy stays false and the chat survives; the // fresh onMount then sees mode is still APP and skips its clearing saveAndClear. let leaveOnDestroy = $state(false) - beforeNavigate(({ to }) => { - // Recompute on every navigation (both branches) so a stale decision from - // an earlier same-app navigation never lingers into a later cross-app one. - leaveOnDestroy = !navStaysInEditor(to?.url.pathname ?? '', '/apps_raw/edit/', path) + beforeNavigate(({ from, to }) => { + // Recompute on every navigation (both branches) so a stale decision never + // lingers. Decided from the route pathnames (not this instance's `path`, + // which is '' on the add page) so cross-app navigation clears reliably. + leaveOnDestroy = !navStaysInEditor( + from?.url.pathname ?? '', + to?.url.pathname ?? '', + '/apps_raw/add', + '/apps_raw/edit/' + ) }) onMount(() => {