From 61a6bad832677c99ee1ffd13784ac9248770905d Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Fri, 2 Oct 2026 00:02:06 +0200 Subject: [PATCH 1/2] Fixed the post editor's held exit hanging when its save never settles no ref Leaving a dirty draft holds the exit while the save on the way out runs, and that decision had no deadline and no rejection handler. A stalled request or a pending sign-in pinned the URL with no dialog, and every later Back, Forward or link exit was swallowed until a reload. A decision that fails, or has not come within 20 seconds, now falls back to the leave confirmation, so the writer always has a way out and Stay keeps the editor. 20 seconds outlasts the transport's 15 seconds of retries on an unreachable or maintenance-mode server plus its final attempt, so a save that is about to land is not cut off. --- .../editor-leave-guard.acceptance.test.tsx | 26 ++++++- apps/admin/src/editor/session/README.md | 5 +- apps/admin/src/editor/session/leave-guard.ts | 13 +++- .../editor/session/use-leave-guard.test.ts | 76 +++++++++++++++++++ .../src/editor/session/use-leave-guard.ts | 4 +- 5 files changed, 119 insertions(+), 5 deletions(-) diff --git a/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx b/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx index 119f80c1521..36b525a5870 100644 --- a/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, onTestFinished } from 'vitest'; +import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import { userEvent } from 'vitest/browser'; import { buildLexicalParagraph } from '@tryghost/test-data'; @@ -16,6 +16,7 @@ import { type RenderAdminAppOptions, } from '@test-utils/acceptance'; import { editorScreen } from '@/editor/editor.screen'; +import { LEAVE_DECISION_DEADLINE_MS } from '@/editor/session/leave-guard'; import { postsListScreen } from '@/posts/list/posts-list.screen'; import { deferred } from '@/utils/deferred'; @@ -368,6 +369,29 @@ describe('Post editor leave guard', () => { await expect.element(editorScreen.body()).toHaveTextContent('Hello from React and more'); }); + it('asks before leaving when the save on the way out never answers', async () => { + const { saveApi, resolveSave } = fakeDeferredSave(); + await openDirtyEditor(withoutAutosave(FLAG_ON)); + // Only the deadline's clock is faked; requests, rendering and polling stay on real time. + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'], shouldClearNativeTimers: true }); + try { + await editorScreen.backLink('post').click(); + await expect.poll(() => saveApi.requests.length).toBe(1); + + vi.advanceTimersByTime(LEAVE_DECISION_DEADLINE_MS); + + await expect.element(editorScreen.leaveDialog()).toBeVisible(); + vi.useRealTimers(); + expect(currentRoute()).toBe(`/editor/post/${POST_ID}`); + await editorScreen.leaveEditor().click(); + await expect.poll(currentRoute).toBe('/posts'); + await expect(editorScreen.root()).toHaveCount(0); + } finally { + vi.useRealTimers(); + resolveSave(); + } + }); + it('guards a native hash anchor out of the editor', async () => { const saveApi = fakeEditablePost({ status: 'published', diff --git a/apps/admin/src/editor/session/README.md b/apps/admin/src/editor/session/README.md index 084be4fff0c..81bf8ea98d3 100644 --- a/apps/admin/src/editor/session/README.md +++ b/apps/admin/src/editor/session/README.md @@ -373,7 +373,10 @@ While the post holds unsaved work, every way out of the editor is put to the save engine: a link, the browser's Back and Forward buttons, and any other change to the URL's hash. The engine finishes or saves what is outstanding and answers either that leaving loses nothing, and the navigation goes ahead, or -that the writer has to confirm it. Until then the URL stays on the editor. A +that the writer has to confirm it. Until then the URL stays on the editor. The +writer is asked to confirm instead when the engine fails to answer or has not +answered within twenty seconds, which is longer than the transport keeps +retrying a save, so a stalled save or a pending sign-in cannot pin the URL. A Back or Forward is undone as it happens and replayed once the writer may leave, so they land on the entry it reached. Undoing it puts the editor back directly above that entry: a held Back drops the forward history, and a Forward or a hash diff --git a/apps/admin/src/editor/session/leave-guard.ts b/apps/admin/src/editor/session/leave-guard.ts index 650ea84f491..70cb8d79d01 100644 --- a/apps/admin/src/editor/session/leave-guard.ts +++ b/apps/admin/src/editor/session/leave-guard.ts @@ -1,7 +1,18 @@ import { withoutTrailingSlash } from '@/hooks/use-history-pop-navigation-guard'; -import type { SaveEngineState } from '@/editor/engine/save-engine'; +import type { LeaveDecision, SaveEngineState } from '@/editor/engine/save-engine'; import type { PostType } from '@/editor/card-config'; +// Outlasts the transport's 15s of retries and its final attempt, so a landing save is not cut off. +export const LEAVE_DECISION_DEADLINE_MS = 20_000; + +/** The engine's answer, or `confirm` once it fails or misses the deadline. */ +export function leaveDecisionWithin(decision: Promise): Promise { + return new Promise((resolve) => { + const deadline = setTimeout(() => resolve('confirm'), LEAVE_DECISION_DEADLINE_MS); + void decision.then(resolve, () => resolve('confirm')).finally(() => clearTimeout(deadline)); + }); +} + interface GuardedLocation { pathname: string; state?: unknown; diff --git a/apps/admin/src/editor/session/use-leave-guard.test.ts b/apps/admin/src/editor/session/use-leave-guard.test.ts index 45563ddaef2..3dd5deeb125 100644 --- a/apps/admin/src/editor/session/use-leave-guard.test.ts +++ b/apps/admin/src/editor/session/use-leave-guard.test.ts @@ -5,6 +5,7 @@ import { createHashRouter, createMemoryRouter, RouterProvider, useLocation } fro import { beforeAll, describe, expect, it, vi } from 'vitest'; import { installHistoryPopGate } from '@/hooks/use-history-pop-navigation-guard'; import { deferred } from '@/utils/deferred'; +import { LEAVE_DECISION_DEADLINE_MS } from './leave-guard'; import { useEditorLeaveGuard, type EditorLeaveGuard } from './use-leave-guard'; import { useEditorSessionKey, type EditorSessionHandle } from './use-editor-session'; @@ -291,4 +292,79 @@ describe('useEditorLeaveGuard', () => { router.dispose(); } }); + + function renderHeldExit(leaveRequested: () => Promise) { + const view: { guard?: EditorLeaveGuard } = {}; + function Editor() { + view.guard = useEditorLeaveGuard( + { + state: { kind: 'saving', intent: 'leave' }, + createdId: null, + isDirty: () => true, + leaveRequested, + } as unknown as EditorSessionHandle, + 'post', + ); + return createElement('main', { 'data-testid': 'editor' }); + } + const router = createMemoryRouter( + [ + { path: '/editor/post/abc', element: createElement(Editor) }, + { path: '/posts', element: 'Posts' }, + ], + { initialEntries: ['/editor/post/abc'] }, + ); + const screen = render(createElement(RouterProvider, { router })); + return { router, screen, guard: () => view.guard! }; + } + + it('asks before leaving once a held exit outlasts the deadline, and Leave still goes', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const { router, guard } = renderHeldExit(() => new Promise(() => {})); + try { + await act(async () => { + await router.navigate('/posts'); + }); + await act(() => vi.advanceTimersByTimeAsync(LEAVE_DECISION_DEADLINE_MS - 1)); + expect(guard().dialogProps.open).toBe(false); + + await act(() => vi.advanceTimersByTimeAsync(1)); + + expect(guard().dialogProps.open).toBe(true); + expect(router.state.location.pathname).toBe('/editor/post/abc'); + vi.useRealTimers(); + act(() => { + guard().dialogProps.onConfirm(); + guard().dialogProps.onOpenChange(false); + }); + await waitFor(() => expect(router.state.location.pathname).toBe('/posts')); + } finally { + vi.useRealTimers(); + router.dispose(); + } + }); + + it('asks before leaving when the leave decision fails, and Stay keeps the editor', async () => { + const leaveRequested = vi.fn().mockRejectedValue(new Error('Leave decision failed')); + const { router, screen, guard } = renderHeldExit(leaveRequested); + try { + await act(async () => { + await router.navigate('/posts'); + }); + + await waitFor(() => expect(guard().dialogProps.open).toBe(true)); + act(() => guard().dialogProps.onOpenChange(false)); + + expect(guard().dialogProps.open).toBe(false); + expect(router.state.location.pathname).toBe('/editor/post/abc'); + expect(screen.getByTestId('editor')).toBeInTheDocument(); + await act(async () => { + await router.navigate('/posts'); + }); + await waitFor(() => expect(guard().dialogProps.open).toBe(true)); + expect(leaveRequested).toHaveBeenCalledTimes(2); + } finally { + router.dispose(); + } + }); }); diff --git a/apps/admin/src/editor/session/use-leave-guard.ts b/apps/admin/src/editor/session/use-leave-guard.ts index ec53f46c383..1446f16e5ec 100644 --- a/apps/admin/src/editor/session/use-leave-guard.ts +++ b/apps/admin/src/editor/session/use-leave-guard.ts @@ -2,7 +2,7 @@ import { useEffect, useRef, useState } from 'react'; import { useLocation, useNavigate } from '@tryghost/admin-x-framework'; import { useUnsavedChangesGuard } from '@/hooks/use-unsaved-changes-guard'; import type { PostType } from '@/editor/card-config'; -import { hasUnsavedWork, isCreatedIdUrlSwap } from './leave-guard'; +import { hasUnsavedWork, isCreatedIdUrlSwap, leaveDecisionWithin } from './leave-guard'; import { useEditorSessionKey, type EditorSessionHandle } from './use-editor-session'; export interface EditorLeaveGuard { @@ -118,7 +118,7 @@ export function useEditorLeaveGuard( return; } isDecidingRef.current = true; - void leaveRequested().then((decision) => { + void leaveDecisionWithin(leaveRequested()).then((decision) => { if (!isMountedRef.current) { return; } From 5b6d6e875c546613bf68ade35ca418599f812346 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Fri, 2 Oct 2026 01:10:53 +0200 Subject: [PATCH 2/2] Fixed the editor's leave deadline interrupting a sign-in no ref When the save on the way out found the session expired, the deadline could open the leave dialog over the sign-in dialog after 20 seconds. Focus moved to Stay, the sign-in stopped taking clicks, and signing in no longer carried the writer out, though signing in or cancelling already gave them a way out. The deadline now waits while the writer signs in again. Once the deadline has asked, a later exit asks at once while the engine is still in the state it timed out in, instead of waiting out the same stall again. The README now says a confirmation the deadline asks for is not reported the way the engine's own confirmations are. --- .../editor-leave-guard.acceptance.test.tsx | 61 +++++++++++++++++++ apps/admin/src/editor/session/README.md | 10 ++- apps/admin/src/editor/session/leave-guard.ts | 24 +++++++- .../editor/session/use-leave-guard.test.ts | 58 +++++++++++++++++- .../src/editor/session/use-leave-guard.ts | 20 +++++- 5 files changed, 164 insertions(+), 9 deletions(-) diff --git a/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx b/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx index 36b525a5870..31b2f866743 100644 --- a/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-leave-guard.acceptance.test.tsx @@ -159,6 +159,44 @@ function fakeDeferredSave() { }; } +/** A draft whose saves find the session gone until `restoreSaves` answers them again. */ +function fakeExpiredSaves() { + fakeEditorChrome(); + const loaded = post({ + id: POST_ID, + title: 'Hello from React', + slug: 'hello-from-react', + status: 'draft', + lexical: buildLexicalParagraph('Hello from React'), + updated_at: LOADED_AT, + published_at: null, + tags: [], + }); + const postRoute = new RegExp(`^/posts/${POST_ID}/\\?`); + fakeAdminEndpoint('GET', /^\/slugs\/post\//, ({ url }) => ({ + slugs: [{ slug: decodeURIComponent(url.split('/slugs/post/')[1].split('/')[0]) }], + })); + fakeAdminEndpoint('GET', postRoute, () => ({ posts: [loaded] })); + const expiredApi = fakeAdminEndpoint( + 'PUT', + postRoute, + { errors: [{ type: 'UnauthorizedError', message: 'Authorization failed' }] }, + { status: 401 }, + ); + + return { + expiredApi, + // Declared after the expired fake, so they take over from it. + restoreSaves: () => { + fakeAdminEndpoint('POST', '/session/', () => 'Created', { status: 201 }); + return fakeAdminEndpoint('PUT', postRoute, ({ body }) => { + const submitted = (body as { posts: Partial[] }).posts[0]; + return { posts: [{ ...loaded, ...submitted, updated_at: '2026-01-01T00:00:01.000Z' }] }; + }); + }, + }; +} + async function appendToBody(text: string) { const body = editorScreen.body(); // One input event: a fast autosave must not split a keyboard sequence into several saves. @@ -392,6 +430,29 @@ describe('Post editor leave guard', () => { } }); + it('lets a sign-in that outlasts the deadline carry the writer out without asking', async () => { + const { expiredApi, restoreSaves } = fakeExpiredSaves(); + await openDirtyEditor(withoutAutosave(FLAG_ON)); + const dialogInsertions = watchLeaveDialog(); + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'], shouldClearNativeTimers: true }); + try { + await editorScreen.backLink('post').click(); + await expect.element(editorScreen.reauthDialog()).toBeVisible(); + expect(expiredApi.requests.length).toBe(1); + + vi.advanceTimersByTime(LEAVE_DECISION_DEADLINE_MS * 2); + } finally { + vi.useRealTimers(); + } + const restoredApi = restoreSaves(); + await editorScreen.reauthPassword().fill('hunter22'); + await editorScreen.reauthSignIn().click(); + + await expect.poll(currentRoute).toBe('/posts'); + expect(restoredApi.requests.length).toBe(1); + expect(dialogInsertions()).toBe(0); + }); + it('guards a native hash anchor out of the editor', async () => { const saveApi = fakeEditablePost({ status: 'published', diff --git a/apps/admin/src/editor/session/README.md b/apps/admin/src/editor/session/README.md index 81bf8ea98d3..4cce59cc20e 100644 --- a/apps/admin/src/editor/session/README.md +++ b/apps/admin/src/editor/session/README.md @@ -376,7 +376,10 @@ answers either that leaving loses nothing, and the navigation goes ahead, or that the writer has to confirm it. Until then the URL stays on the editor. The writer is asked to confirm instead when the engine fails to answer or has not answered within twenty seconds, which is longer than the transport keeps -retrying a save, so a stalled save or a pending sign-in cannot pin the URL. A +retrying a save, so a stalled save cannot pin the URL. The deadline does not run +out while the writer is signing in again: signing in lets the leave go ahead, +and cancelling asks. Once the deadline has run out, the next way out asks at +once until the engine moves on. A Back or Forward is undone as it happens and replayed once the writer may leave, so they land on the entry it reached. Undoing it puts the editor back directly above that entry: a held Back drops the forward history, and a Forward or a hash @@ -393,8 +396,9 @@ request that ran and failed is reported once, with the command it ran, the error, whether the post already had a server id, the post's persisted status, the id, and how long the request took. Queued work a failure dropped is not reported on its own. An expired session is reported only when re-authentication -is abandoned, not when it is retried. A leave the writer has to -confirm is reported with the reason codes the tracker holds the post dirty for. +is abandoned, not when it is retried. A leave the engine answers with a +confirmation is reported with the reason codes the tracker holds the post dirty +for; one the editor asks about because the engine missed its deadline is not. A draft disposed with a title but a slug still derived from the default title is reported as an error. A throwing subscriber or slug listener is reported as an error, and so is a slug edit the generator rejected. A local copy that storage diff --git a/apps/admin/src/editor/session/leave-guard.ts b/apps/admin/src/editor/session/leave-guard.ts index 70cb8d79d01..230a67306b0 100644 --- a/apps/admin/src/editor/session/leave-guard.ts +++ b/apps/admin/src/editor/session/leave-guard.ts @@ -5,10 +5,30 @@ import type { PostType } from '@/editor/card-config'; // Outlasts the transport's 15s of retries and its final attempt, so a landing save is not cut off. export const LEAVE_DECISION_DEADLINE_MS = 20_000; +interface LeaveDeadline { + ms: number; + /** While true the deadline waits, because signing in again decides the leave. */ + isSigningIn: () => boolean; + /** Called when the deadline answers rather than the engine. */ + onLate: () => void; +} + /** The engine's answer, or `confirm` once it fails or misses the deadline. */ -export function leaveDecisionWithin(decision: Promise): Promise { +export function leaveDecisionWithin( + decision: Promise, + { ms, isSigningIn, onLate }: LeaveDeadline, +): Promise { return new Promise((resolve) => { - const deadline = setTimeout(() => resolve('confirm'), LEAVE_DECISION_DEADLINE_MS); + let deadline: ReturnType; + const expire = () => { + if (isSigningIn()) { + deadline = setTimeout(expire, LEAVE_DECISION_DEADLINE_MS); + return; + } + onLate(); + resolve('confirm'); + }; + deadline = setTimeout(expire, ms); void decision.then(resolve, () => resolve('confirm')).finally(() => clearTimeout(deadline)); }); } diff --git a/apps/admin/src/editor/session/use-leave-guard.test.ts b/apps/admin/src/editor/session/use-leave-guard.test.ts index 3dd5deeb125..7067f317e39 100644 --- a/apps/admin/src/editor/session/use-leave-guard.test.ts +++ b/apps/admin/src/editor/session/use-leave-guard.test.ts @@ -5,6 +5,7 @@ import { createHashRouter, createMemoryRouter, RouterProvider, useLocation } fro import { beforeAll, describe, expect, it, vi } from 'vitest'; import { installHistoryPopGate } from '@/hooks/use-history-pop-navigation-guard'; import { deferred } from '@/utils/deferred'; +import type { SaveEngineState } from '@/editor/engine/save-engine'; import { LEAVE_DECISION_DEADLINE_MS } from './leave-guard'; import { useEditorLeaveGuard, type EditorLeaveGuard } from './use-leave-guard'; import { useEditorSessionKey, type EditorSessionHandle } from './use-editor-session'; @@ -293,12 +294,15 @@ describe('useEditorLeaveGuard', () => { } }); - function renderHeldExit(leaveRequested: () => Promise) { + function renderHeldExit( + leaveRequested: () => Promise, + state: SaveEngineState = { kind: 'saving', intent: 'leave' }, + ) { const view: { guard?: EditorLeaveGuard } = {}; function Editor() { view.guard = useEditorLeaveGuard( { - state: { kind: 'saving', intent: 'leave' }, + state, createdId: null, isDirty: () => true, leaveRequested, @@ -344,6 +348,56 @@ describe('useEditorLeaveGuard', () => { } }); + it('asks at once on the next exit while the engine is still where the deadline left it', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const { router, guard } = renderHeldExit(() => new Promise(() => {})); + try { + await act(async () => { + await router.navigate('/posts'); + }); + await act(() => vi.advanceTimersByTimeAsync(LEAVE_DECISION_DEADLINE_MS)); + expect(guard().dialogProps.open).toBe(true); + act(() => guard().dialogProps.onOpenChange(false)); + + await act(async () => { + await router.navigate('/posts'); + }); + await act(() => vi.advanceTimersByTimeAsync(0)); + + expect(guard().dialogProps.open).toBe(true); + } finally { + vi.useRealTimers(); + router.dispose(); + } + }); + + it('waits out a sign-in that outlasts the deadline, and lets it decide the exit', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const decision = deferred<'proceed' | 'confirm'>(); + const { router, guard } = renderHeldExit(() => decision.promise, { + kind: 'reauth-pending', + intent: 'leave', + }); + try { + await act(async () => { + await router.navigate('/posts'); + }); + await act(() => vi.advanceTimersByTimeAsync(LEAVE_DECISION_DEADLINE_MS * 3)); + expect(guard().dialogProps.open).toBe(false); + + await act(async () => { + decision.resolve('proceed'); + await decision.promise; + }); + + expect(router.state.location.pathname).toBe('/posts'); + expect(guard().dialogProps.open).toBe(false); + } finally { + vi.useRealTimers(); + router.dispose(); + } + }); + it('asks before leaving when the leave decision fails, and Stay keeps the editor', async () => { const leaveRequested = vi.fn().mockRejectedValue(new Error('Leave decision failed')); const { router, screen, guard } = renderHeldExit(leaveRequested); diff --git a/apps/admin/src/editor/session/use-leave-guard.ts b/apps/admin/src/editor/session/use-leave-guard.ts index 1446f16e5ec..69d8c31d230 100644 --- a/apps/admin/src/editor/session/use-leave-guard.ts +++ b/apps/admin/src/editor/session/use-leave-guard.ts @@ -2,7 +2,13 @@ import { useEffect, useRef, useState } from 'react'; import { useLocation, useNavigate } from '@tryghost/admin-x-framework'; import { useUnsavedChangesGuard } from '@/hooks/use-unsaved-changes-guard'; import type { PostType } from '@/editor/card-config'; -import { hasUnsavedWork, isCreatedIdUrlSwap, leaveDecisionWithin } from './leave-guard'; +import type { SaveEngineState } from '@/editor/engine/save-engine'; +import { + hasUnsavedWork, + isCreatedIdUrlSwap, + LEAVE_DECISION_DEADLINE_MS, + leaveDecisionWithin, +} from './leave-guard'; import { useEditorSessionKey, type EditorSessionHandle } from './use-editor-session'; export interface EditorLeaveGuard { @@ -44,6 +50,10 @@ export function useEditorLeaveGuard( const guardRef = useRef(guard); guardRef.current = guard; + const stateRef = useRef(session.state); + stateRef.current = session.state; + // The engine state a leave last ran out of time in: another leave in that state asks at once. + const lateStateRef = useRef(null); // Stays set while an accepted exit is transitioning. React Router does not // consult blockers again in its `proceeding` state, so the deferred ID swap // must not race and replace that navigation either. @@ -118,7 +128,13 @@ export function useEditorLeaveGuard( return; } isDecidingRef.current = true; - void leaveDecisionWithin(leaveRequested()).then((decision) => { + void leaveDecisionWithin(leaveRequested(), { + ms: stateRef.current === lateStateRef.current ? 0 : LEAVE_DECISION_DEADLINE_MS, + isSigningIn: () => stateRef.current.kind === 'reauth-pending', + onLate: () => { + lateStateRef.current = stateRef.current; + }, + }).then((decision) => { if (!isMountedRef.current) { return; }