From 1bdbd94df6da554ed53aa5cbe322348c234e3cae Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 12:11:23 -0700 Subject: [PATCH] Keep draft delivery recoverable across stalled reads and edits --- .../creation-draft-delivery.test.ts | 36 +++++++++++++ .../creation-draft-delivery.ts | 18 ++++--- .../creation-draft-readiness.ts | 41 +++++++-------- .../creation-draft-submit.test.ts | 51 ++++++++++++++++++- .../creation-draft-submit.ts | 7 ++- 5 files changed, 119 insertions(+), 34 deletions(-) diff --git a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.test.ts b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.test.ts index a589500a59d..ed66c6bb52b 100644 --- a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.test.ts +++ b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.test.ts @@ -79,6 +79,42 @@ describe('explicit creation draft delivery', () => { vi.unstubAllGlobals() }) + it.each(['capabilities', 'listing'])( + 'bounds a hung %s query without sending late', + async (query) => { + let respond!: (value: unknown) => void + const hanging = new Promise((resolve) => { + respond = resolve + }) + if (query === 'capabilities') { + mocks.capabilities.mockReturnValue(hanging) + } else { + mocks.rpc.mockReturnValue(hanging) + } + const pending = sendCreationDraft({ target, text: 'hello' }) + await vi.advanceTimersByTimeAsync(1000) + expect(await pending).toMatchObject({ status: 'refused' }) + respond(query === 'capabilities' ? [TERMINAL_SEND_INCARNATION_RUNTIME_CAPABILITY] : listing()) + await vi.runAllTimersAsync() + expect(sends()).toEqual([]) + expect(vi.getTimerCount()).toBe(0) + } + ) + + it.each(['capabilities', 'listing'])('bounds capture when %s never responds', async (query) => { + const hanging = new Promise(() => {}) + if (query === 'capabilities') { + mocks.capabilities.mockReturnValue(hanging) + } else { + mocks.rpc.mockReturnValue(hanging) + } + const pending = captureCreationDraftTarget(target) + await vi.advanceTimersByTimeAsync(1000) + expect(await pending).toBeNull() + expect(sends()).toEqual([]) + expect(vi.getTimerCount()).toBe(0) + }) + it('refuses startup without writing any bytes', async () => { mocks.ready.mockResolvedValue(false) expect(await deliver()).toEqual({ status: 'refused', reason: 'input-not-ready' }) diff --git a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.ts b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.ts index 292b3bd99ac..ad1c3e5908f 100644 --- a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.ts +++ b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-delivery.ts @@ -1,4 +1,5 @@ import { useAppStore } from '@/store' +import { withTimeout } from '../../../../shared/promise-timeout-fallback' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { refreshLocalRuntimeCapabilities } from '@/runtime/local-runtime-capabilities' import { runTerminalPtyInputTransaction } from '@/components/terminal-pane/terminal-pty-input-transaction' @@ -61,14 +62,17 @@ async function readNativeTarget(target: UnverifiedDraftTarget) { return null } try { - const listing = await callRuntimeRpc( - { kind: 'local' }, - 'terminal.list', - { + const listing = await withTimeout( + callRuntimeRpc({ kind: 'local' }, 'terminal.list', { handles: [target.terminalHandle], includeVisualLayouts: false - } + }), + 1000, + null ) + if (!listing) { + return null + } const matches = listing.terminals.filter((entry) => entry.handle === target.terminalHandle) const terminal = matches.length === 1 ? matches[0] : undefined return terminal?.ptyId && @@ -94,7 +98,7 @@ export async function captureCreationDraftTarget(args: UnverifiedDraftTarget): P return null } if ( - !(await refreshLocalRuntimeCapabilities()).includes( + !(await withTimeout(refreshLocalRuntimeCapabilities(), 1000, [])).includes( TERMINAL_SEND_INCARNATION_RUNTIME_CAPABILITY ) ) { @@ -132,7 +136,7 @@ export async function sendCreationDraft(args: { return { status: 'refused', reason: 'invalid-text' } } if ( - !(await refreshLocalRuntimeCapabilities()).includes( + !(await withTimeout(refreshLocalRuntimeCapabilities(), 1000, [])).includes( TERMINAL_SEND_INCARNATION_RUNTIME_CAPABILITY ) ) { diff --git a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-readiness.ts b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-readiness.ts index 035b0afd27a..d23eccc566a 100644 --- a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-readiness.ts +++ b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-readiness.ts @@ -1,30 +1,23 @@ import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import type { RuntimeTerminalWait } from '../../../../shared/runtime-types' +import { withTimeout } from '../../../../shared/promise-timeout-fallback' /** Uses host-retained state so mounting xterm cannot consume our readiness evidence. */ export async function isCreationDraftInputReady(terminalHandle: string): Promise { - let timer: ReturnType | undefined - try { - return await Promise.race([ - callRuntimeRpc<{ wait: RuntimeTerminalWait }>({ kind: 'local' }, 'terminal.wait', { - terminal: terminalHandle, - for: 'tui-idle', - timeoutMs: 100 - }).then( - ({ wait }) => - wait?.handle === terminalHandle && - wait.condition === 'tui-idle' && - wait.status === 'running' && - wait.satisfied === true && - wait.blockedReason === undefined - ), - new Promise((resolve) => { - timer = setTimeout(() => resolve(false), 1000) - }) - ]) - } catch { - return false - } finally { - clearTimeout(timer) - } + return withTimeout( + callRuntimeRpc<{ wait: RuntimeTerminalWait }>({ kind: 'local' }, 'terminal.wait', { + terminal: terminalHandle, + for: 'tui-idle', + timeoutMs: 100 + }).then( + ({ wait }) => + wait?.handle === terminalHandle && + wait.condition === 'tui-idle' && + wait.status === 'running' && + wait.satisfied === true && + wait.blockedReason === undefined + ), + 1000, + false + ) } diff --git a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.test.ts b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.test.ts index c0474b73e30..d6e2037c465 100644 --- a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.test.ts +++ b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.test.ts @@ -121,7 +121,7 @@ describe('creation draft explicit delivery', () => { expect(delivery.sendCreationDraft).toHaveBeenCalledOnce() }) - it('retains edits made while sending and never replaces them with the sent text', async () => { + it('leaves remount edits unsent when an older version finishes sending', async () => { const sending = deferred<{ status: 'delivered' }>() delivery.sendCreationDraft.mockReturnValue(sending.promise) const submitting = submitCreationDraft(source.id) @@ -129,9 +129,56 @@ describe('creation draft explicit delivery', () => { editCreationDraft({ ...current().buffer, text: 'Newer source' }) sending.resolve({ status: 'delivered' }) await submitting - expect(durable).toMatchObject({ text: 'Newer source', delivery: { state: 'delivered' } }) + expect(durable).toMatchObject({ text: 'Newer source' }) + expect(durable?.delivery).toBeUndefined() + expect(delivery.sendCreationDraft).toHaveBeenCalledOnce() + expect(delivery.sendCreationDraft.mock.calls[0][0].text).toBe(source.text) + expect(await submitCreationDraft(source.id)).toEqual({ status: 'delivered' }) + expect(delivery.sendCreationDraft).toHaveBeenCalledTimes(2) + expect(delivery.sendCreationDraft.mock.calls[1][0].text).toBe('Newer source') + expect(durable?.delivery?.state).toBe('delivered') }) + it('retains the uncertain fence when text changes during an unconfirmed send', async () => { + const sending = deferred<{ status: 'uncertain'; reason: 'transport' }>() + delivery.sendCreationDraft.mockReturnValue(sending.promise) + const submitting = submitCreationDraft(source.id) + await vi.waitFor(() => expect(delivery.sendCreationDraft).toHaveBeenCalledOnce()) + editCreationDraft({ ...current().buffer, text: 'Newer source' }) + sending.resolve({ status: 'uncertain', reason: 'transport' }) + await submitting + expect(durable).toMatchObject({ text: 'Newer source', delivery: { state: 'uncertain' } }) + expect(await submitCreationDraft(source.id)).toEqual({ status: 'already-attempted' }) + expect(delivery.sendCreationDraft).toHaveBeenCalledOnce() + }) + + it.each(['sending', 'uncertain'] as const)( + 'keeps a restored %s fence after editing without replaying the attempt', + async (state) => { + editCreationDraft({ + ...source, + delivery: { attemptId: 'previous-renderer', revision: 1, state } + }) + await flushCreationDraft(source.id) + const { revision, ...buffer } = durable! + useCreationDraftSession.setState({ + entries: { + [source.id]: { + buffer, + storedRevision: revision, + editVersion: 0, + savedVersion: 0, + error: null + } + } + }) + editCreationDraft({ ...current().buffer, text: 'Edited recovered source' }) + expect(await submitCreationDraft(source.id)).toEqual({ status: 'already-attempted' }) + expect(delivery.sendCreationDraft).not.toHaveBeenCalled() + expect(durable).toMatchObject({ text: 'Edited recovered source', delivery: { state } }) + } + ) + it('refuses sending when another window has already committed a delivery attempt', async () => { durable = { ...durable!, diff --git a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.ts b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.ts index 17b98751a65..6fce0a3a98f 100644 --- a/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.ts +++ b/src/renderer/src/lib/workspace-creation-drafts/creation-draft-submit.ts @@ -91,9 +91,14 @@ async function submit(id: string): Promise { } const current = useCreationDraftSession.getState().entries[id] if (current?.buffer.delivery?.attemptId === attemptId) { + const newerText = current.buffer.text !== buffer.text editCreationDraft({ ...current.buffer, - delivery: result.status === 'refused' ? undefined : { ...delivery, state: result.status }, + // Confirmed delivery belongs to the submitted text, not edits made after a remount. + delivery: + result.status === 'refused' || (result.status === 'delivered' && newerText) + ? undefined + : { ...delivery, state: result.status }, updatedAt: Date.now() }) await flushCreationDraft(id)