From b3284600e4dfff536e140612fc738fbb13892dec Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sat, 5 Sep 2026 19:12:22 -0700 Subject: [PATCH] fix(native-chat): stop an advisory refresh ending the click, and one click per row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual QA found the reveal never ran: the inventory refresh that precedes it is an optimization, but its failure returned early with 'not available yet, retry in a moment' — reinstating the dead end this PR removes, one step earlier. A failed refresh now falls through to the reveal, which is the repair and does not need the refresh to have worked. The click can chain a refresh, a capability probe, a reveal and a second refresh, each with its own timeout, while nothing on the row says it is working. A per-session in-flight guard keeps an impatient second click from running the whole sequence again and landing its own toast. Also drops an unreachable owner scope: the snapshot apply discards any worktree whose execution host is not local before it reads one, so naming a remote scope there described a synchronisation that cannot happen. --- ...tivate-ai-vault-structured-session.test.ts | 35 ++++++++++++ .../activate-ai-vault-structured-session.ts | 57 +++++++++++++++---- 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/lib/activate-ai-vault-structured-session.test.ts b/src/renderer/src/lib/activate-ai-vault-structured-session.test.ts index 922ef7046ee..fd63c28ba61 100644 --- a/src/renderer/src/lib/activate-ai-vault-structured-session.test.ts +++ b/src/renderer/src/lib/activate-ai-vault-structured-session.test.ts @@ -77,6 +77,21 @@ describe('activateAiVaultStructuredSession', () => { expect(parts.gone).not.toHaveBeenCalled() }) + it('still reveals when the inventory refresh itself fails', async () => { + // The refresh is an optimization. Letting its failure end the click reinstates the dead end + // this whole path exists to remove: a closed chat, and advice to retry that cannot come true. + const parts = deps({ + activate: vi.fn().mockReturnValueOnce(false).mockReturnValue(true), + refresh: vi.fn().mockRejectedValueOnce(new Error('structured_session_restore_timeout')) + }) + + await expect(activateAiVaultStructuredSession(structuredSession, parts)).resolves.toBe(true) + + expect(parts.reveal).toHaveBeenCalledOnce() + expect(parts.unavailable).not.toHaveBeenCalled() + expect(parts.gone).not.toHaveBeenCalled() + }) + it('does not strand the click when the post-reveal refresh fails', async () => { const parts = deps({ activate: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(false).mockReturnValue(true), @@ -123,6 +138,26 @@ describe('activateAiVaultStructuredSession', () => { expect(parts.hostCannotOpen).not.toHaveBeenCalled() }) + it('runs one activation per session however many times the row is clicked', async () => { + // Three clicks on a slow row used to run three full sequences and land three toasts. + let release!: (outcome: 'gone') => void + const pending = new Promise<'gone'>((resolve) => { + release = resolve + }) + const parts = deps({ activate: vi.fn(() => false), reveal: vi.fn(() => pending) }) + + const clicks = [ + activateAiVaultStructuredSession(structuredSession, parts), + activateAiVaultStructuredSession(structuredSession, parts), + activateAiVaultStructuredSession(structuredSession, parts) + ] + release('gone') + await Promise.all(clicks) + + expect(parts.reveal).toHaveBeenCalledOnce() + expect(parts.gone).toHaveBeenCalledOnce() + }) + it('ignores a row that is not a structured chat', async () => { const parts = deps() diff --git a/src/renderer/src/lib/activate-ai-vault-structured-session.ts b/src/renderer/src/lib/activate-ai-vault-structured-session.ts index 73d792581c7..69d95eef2db 100644 --- a/src/renderer/src/lib/activate-ai-vault-structured-session.ts +++ b/src/renderer/src/lib/activate-ai-vault-structured-session.ts @@ -88,15 +88,35 @@ export async function activateAiVaultStructuredSession( if (!structured) { return false } + // Why: the click can chain a refresh, a capability probe, a reveal and a second refresh, each + // with its own timeout, and nothing on the row says it is working. Without this, an impatient + // second click runs the whole sequence again and lands its own toast. + const inFlight = activationsInFlight.get(structured.sessionId) + if (inFlight) { + return inFlight + } + const activation = activateStructuredSession(structured, deps) + activationsInFlight.set(structured.sessionId, activation) + try { + return await activation + } finally { + activationsInFlight.delete(structured.sessionId) + } +} + +const activationsInFlight = new Map>() + +async function activateStructuredSession( + structured: NonNullable, + deps: StructuredSessionActivationDeps +): Promise { const target = { worktreeId: structured.workspaceId, sessionId: structured.sessionId } if (!deps.activate(target)) { - try { - await deps.refresh(structured.workspaceId) - } catch { - deps.unavailable() - return true - } - if (!deps.activate(target)) { + // A refresh alone can answer, and costs one call instead of two. It is only ever an + // optimization, so a refresh that fails must fall through to the reveal rather than end the + // click: the host republishing the tab is the repair, and it does not need this to have worked. + const refreshed = await refreshedWithoutThrowing(deps, structured.workspaceId) + if (!refreshed || !deps.activate(target)) { // The inventory genuinely does not carry this chat: it was closed, or this process never // published it. Ask the host to republish the tab from the record it still holds on disk. const revealed = await deps.reveal(target) @@ -111,7 +131,7 @@ export async function activateAiVaultStructuredSession( } return true } - await deps.refresh(structured.workspaceId).catch(() => undefined) + await refreshedWithoutThrowing(deps, structured.workspaceId) if (!deps.activate(target)) { deps.unavailable() return true @@ -124,6 +144,20 @@ export async function activateAiVaultStructuredSession( return true } +/** The inventory refresh is advisory at every call site here, so its failure is a `false`, never a + * thrown end to the click. */ +async function refreshedWithoutThrowing( + deps: StructuredSessionActivationDeps, + worktreeId: string +): Promise { + try { + await deps.refresh(worktreeId) + return true + } catch { + return false + } +} + /** * Ask the host to republish a persisted chat's tab. * @@ -196,10 +230,9 @@ async function refreshStructuredSessionTabs(worktreeId: string): Promise { { timeoutMs: STRUCTURED_SESSION_RESTORE_TIMEOUT_MS } ) ) - applyStructuredSessionTabSnapshots( - [snapshot], - environmentId ? `structured-session:${environmentId}` : undefined - ) + // No owner scope: the apply discards any worktree whose execution host is not local before it + // reads one, so a paired workspace is carried by the subscription, not by this call. + applyStructuredSessionTabSnapshots([snapshot]) } async function withStructuredSessionRestoreTimeout(promise: Promise): Promise {