From a4b48064b1b615af9fc56cddfacae99725cf890b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:52:05 -0700 Subject: [PATCH] fix(desktop): move the pre-session draft only when the fresh chat is re-homed, keyed on the scope swap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The salvaged guard migrated the `__new__` bucket when the composer's runtime session id went from empty to set in the same render as the scope change. That misses the reporter's path and over-reaches on another: - Cold-start resume-last-session (use-desktop-integrations) navigates to the remembered route as soon as the session list loads; the route flips the composer scope immediately while `session.resume` publishes the runtime id later, so the guard saw an empty id and never migrated — the typed text vanished exactly as reported. - Opening an existing session from a fresh chat also goes empty → set, so the guard would carry the new-chat draft into whatever session the user clicked. Drafts are per scope by design (the id-rotation migration in chat/index.tsx is deliberately same-conversation only); a sidebar click must leave the new-chat draft in its bucket. The composer swap cannot tell the two apart from its props, so the site that assigns the id says so: `announceNewSessionDraftKey(stored)` at the two seams where a fresh chat becomes a stored session (first-send `session.create` in createBackendSessionForSend, cold-start restore of the remembered session / route), consumed once by the swap effect via `adoptNewSessionDraft(scope)`, which moves the bucket after the outgoing cleanup stashed the live editor text under it and before the incoming scope is restored — so the text follows the chat with no duplicate left in `__new__`. The existing helper still refuses to overwrite a non-empty destination. The `'active'` sentinel in use-composer-draft.ts is a focus-routing target (`markActiveComposer`), never a draft key, so it needs no migration. Tests: the hook test now flips the scope with the runtime id still unknown (red on origin/main and on the salvaged guard alone) plus a control that a plain new-chat → existing-session switch leaves the bucket untouched (red on the salvaged guard alone). The store-level duplicate of the migration test is dropped; the non-empty-destination invariant stays. --- .../hooks/use-composer-draft.test.tsx | 45 +++++++++++++------ .../chat/composer/hooks/use-composer-draft.ts | 23 +++++----- .../contrib/hooks/use-desktop-integrations.ts | 7 +++ .../hooks/use-session-actions/index.ts | 5 ++- apps/desktop/src/store/composer.test.ts | 13 ------ apps/desktop/src/store/composer.ts | 24 ++++++++++ 6 files changed, 77 insertions(+), 40 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx index 5bee360192..c413d81be7 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx @@ -3,7 +3,14 @@ import { useLayoutEffect } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import { PaneVisibleContext } from '@/components/pane-shell/pane-visibility' -import { clearSessionDraft, type ComposerAttachment, mainComposerScope, stashSessionDraft, takeSessionDraft } from '@/store/composer' +import { + announceNewSessionDraftKey, + clearSessionDraft, + type ComposerAttachment, + mainComposerScope, + stashSessionDraft, + takeSessionDraft +} from '@/store/composer' import { $connection } from '@/store/session' import { useComposerActions } from '../../hooks/use-composer-actions' @@ -94,7 +101,7 @@ describe('useComposerDraft — attachment scope stays coherent with the committe expect(snapshots[0]).toEqual([]) }) - it('moves a new-chat draft into the assigned session when its id arrives (#114122)', () => { + it('carries a pre-session draft onto the session the fresh chat is re-homed to, before its runtime id is known', () => { const preSessionAttachment: ComposerAttachment = { id: 'file:new', kind: 'file', label: 'new.txt' } stashSessionDraft(null, 'do not lose this draft', [preSessionAttachment]) @@ -102,21 +109,33 @@ describe('useComposerDraft — attachment scope stays coherent with the committe undefined} sessionId="" /> ) + // Cold-start resume-last-session / first-send create: the route flips the + // composer scope while `session.resume` has not published a runtime id yet. + announceNewSessionDraftKey('session-created') act(() => { - rerender( - undefined} - sessionId="session-created" - /> - ) + rerender( undefined} sessionId="" />) }) - expect(takeSessionDraft('session-created')).toEqual({ - attachments: [preSessionAttachment], - text: 'do not lose this draft' - }) + expect(mainComposerScope.$attachments.get()).toEqual([preSessionAttachment]) + expect(takeSessionDraft('session-created')).toEqual({ attachments: [preSessionAttachment], text: 'do not lose this draft' }) expect(takeSessionDraft(null)).toEqual({ attachments: [], text: '' }) + clearSessionDraft('session-created') + }) + + it('leaves the pre-session draft in its bucket when the user opens another session from a fresh chat', () => { + stashSessionDraft(null, 'still composing a new chat', []) + + const { rerender } = render( + undefined} sessionId="" /> + ) + + act(() => { + rerender( undefined} sessionId="session-A" />) + }) + + expect(takeSessionDraft('session-A')).toEqual({ attachments: [], text: '' }) + expect(takeSessionDraft(null).text).toBe('still composing a new chat') + clearSessionDraft(null) }) it('applies a delayed image preview when it resolves while its attachment draft is inactive', async () => { diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts index fb69338790..423572b1b6 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts @@ -14,9 +14,9 @@ import { isElementInHiddenPane } from '@/components/pane-shell/pane-visibility' import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { useStoreSelector } from '@/lib/use-session-slice' import { + adoptNewSessionDraft, type ComposerAttachment, type ComposerDraftSyncMode, - migrateSessionDraft, onComposerDraftSyncRequest, reloadPersistedDrafts, stashSessionDraft, @@ -128,9 +128,6 @@ export function useComposerDraft({ const draftScopeRef = useRef(activeQueueSessionKey) const sessionIdRef = useRef(sessionId) sessionIdRef.current = sessionId - // Updated by the draft-swap layout effect so it remains the previous committed - // id during the render where a new chat receives its first session id. - const committedSessionIdRef = useRef(sessionId) const queueEditStateRef = useRef(queueEditRef.current) queueEditStateRef.current = queueEditRef.current @@ -457,19 +454,19 @@ export function useComposerDraft({ // fire later would just clobber with an older snapshot. window.clearTimeout(draftPersistTimerRef.current) pendingDraftPersistRef.current = null - const previousDraftScope = draftScopeRef.current - const previousSessionId = committedSessionIdRef.current - // A new chat writes to the shared pre-session bucket until its first - // runtime session id arrives. Move that draft at this handoff, before the - // incoming scope is restored. Do not consume the bucket on an ordinary - // initial mount of an existing session or on a cross-session switch. - if (!previousSessionId && sessionId && !previousDraftScope && activeQueueSessionKey) { - migrateSessionDraft(previousDraftScope, activeQueueSessionKey) + // A new chat writes to the shared pre-session bucket until its stored id + // arrives; the assigning site announces that id (store/composer.ts). Move + // the bucket at this handoff — after the outgoing cleanup stashed the live + // editor text under it, before the incoming scope is restored — so the + // text the user kept typing follows the chat instead of vanishing. + // Keyed on the scope alone: the runtime id can land a resume later than + // the route flips the scope, so it is not a usable signal here. + if (!draftScopeRef.current && activeQueueSessionKey) { + adoptNewSessionDraft(activeQueueSessionKey) } draftScopeRef.current = activeQueueSessionKey - committedSessionIdRef.current = sessionId const { attachments, text } = takeSessionDraft(activeQueueSessionKey) loadIntoComposer(text, attachments) diff --git a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts index a39d7d30c9..a2d72ed60e 100644 --- a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts +++ b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts @@ -8,6 +8,7 @@ import { $diskPluginsScanPending } from '@/contrib/runtime-loader' import { resolveDeepLinkAction } from '@/lib/deeplink-routes' import { pathFromHermesDeepLink, resolveHermesOpenPath } from '@/lib/hermes-open-target' import { storedSessionIdForNotification } from '@/lib/session-ids' +import { announceNewSessionDraftKey } from '@/store/composer' import { requestMcpInstallFromDeepLink } from '@/store/mcp-deeplink-install' import { startMcpHealthChecker, stopMcpHealthChecker } from '@/store/mcp-health' import { @@ -22,6 +23,7 @@ import { $selectedStoredSessionId, getRememberedRoute, getRememberedSessionId, + resolveComposerSessionKey, sessionBelongsToProfile, setRememberedRoute, setRememberedSessionId @@ -164,6 +166,10 @@ export function useDesktopIntegrations({ !isOverlayView(appViewForPath(route)) && (!routeSession || sessionBelongsToProfile(sessions, routeSession, activeProfile)) ) { + // The user may have started typing on the fresh chat while the + // backend was still coming up; the composer moves that draft onto + // the restored session when its scope swaps (#114122). + announceNewSessionDraftKey(routeSession && resolveComposerSessionKey(routeSession, sessions)) navigate(route, { replace: true }) return @@ -176,6 +182,7 @@ export function useDesktopIntegrations({ } if (last && sessionBelongsToProfile(sessions, last, activeProfile)) { + announceNewSessionDraftKey(resolveComposerSessionKey(last, sessions)) navigate(sessionRoute(last), { replace: true }) return diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index a67815115e..d211801b9b 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -27,7 +27,7 @@ import { isMissingRpcMethod } from '@/lib/gateway-rpc' import { recoverInFlightTurnJournal } from '@/lib/inflight-turn-journal' import { setSessionYolo } from '@/lib/yolo-session' import { $clarifyRequests } from '@/store/clarify' -import { migrateSessionDraft } from '@/store/composer' +import { announceNewSessionDraftKey, migrateSessionDraft } from '@/store/composer' import { clearQueuedPrompts, migrateQueuedPrompts } from '@/store/composer-queue' import { $connectionRequests } from '@/store/connection-request' import { @@ -714,6 +714,9 @@ export function useSessionActions({ // The row carries the create route's exact owner (backend profile + // connection), never the ambient profile — see upsertOptimisticSession. upsertOptimisticSession(created, stored, null, preview?.trim() || null, null, undefined, capturedRoute) + // Anything still parked under the pre-session draft bucket belongs + // to this chat now (#114122); the composer moves it on scope swap. + announceNewSessionDraftKey(stored) navigate(sessionRoute(stored), { replace: true }) // Other windows (e.g. the main window when this is the pop-out) can't // see this session until they re-pull the shared list. diff --git a/apps/desktop/src/store/composer.test.ts b/apps/desktop/src/store/composer.test.ts index db389b09a6..0055e5ae33 100644 --- a/apps/desktop/src/store/composer.test.ts +++ b/apps/desktop/src/store/composer.test.ts @@ -280,19 +280,6 @@ describe('session drafts', () => { clearSessionDraft(tipAfter) }) - it('migrates a pre-session draft onto its assigned session key', () => { - stashSessionDraft(null, 'typed before the session existed', [attachment({ id: 'file:new' })]) - - expect(migrateSessionDraft(null, 'session-created')).toBe(true) - expect(takeSessionDraft('session-created')).toEqual({ - attachments: [attachment({ id: 'file:new' })], - text: 'typed before the session existed' - }) - expect(takeSessionDraft(null)).toEqual({ attachments: [], text: '' }) - - clearSessionDraft('session-created') - }) - it('does not overwrite a destination draft or its attachments during migration', () => { const destinationAttachment = attachment({ id: 'file:destination' }) stashSessionDraft(null, 'new chat draft', [attachment({ id: 'file:source' })]) diff --git a/apps/desktop/src/store/composer.ts b/apps/desktop/src/store/composer.ts index 0981d4bc31..bdaa5d5083 100644 --- a/apps/desktop/src/store/composer.ts +++ b/apps/desktop/src/store/composer.ts @@ -439,6 +439,30 @@ export function migrateSessionDraft(fromKey: string | null | undefined, toKey: s return true } +/** + * The stored id the pre-session chat is about to be re-homed onto, announced + * by the site that assigns it (first-send `session.create`, cold-start + * resume-last-session) and consumed by the composer's scope swap. + * + * The swap cannot tell an assignment apart from the user opening another + * session from a new chat — both flip the scope from the `__new__` bucket to + * a concrete id — and only the assignment may carry the draft along: a + * sidebar click keeps per-scope drafts where they were typed. + */ +let announcedNewSessionDraftKey: string | null = null + +export function announceNewSessionDraftKey(toKey: string | null | undefined): void { + announcedNewSessionDraftKey = toKey?.trim() || null +} + +/** Consume the announcement; move the `__new__` draft when it names `toKey`. */ +export function adoptNewSessionDraft(toKey: string | null | undefined): boolean { + const announced = announcedNewSessionDraftKey + announcedNewSessionDraftKey = null + + return !!announced && announced === toKey?.trim() && migrateSessionDraft(null, toKey) +} + export function setComposerDraft(value: string) { $composerDraft.set(value) }