From 19cabc72b0dbeff95f304746e2da629456631c39 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 04:12:04 -0700 Subject: [PATCH] Revert "fix(notes): release a gone chat's notes, and clear sent ones in the web client too" This reverts commit ae0b9f3fea67d3073be8c8336335ad7527ed65b1. --- .../src/app-shell/AppBackgroundServices.tsx | 5 - .../notes-delivered-by-chat-install.test.ts | 103 ---------- .../src/lib/notes-delivered-by-chat.ts | 16 +- src/renderer/src/main.tsx | 2 + .../apply-preparation-browser.ts | 18 +- .../host-removed-structured-chats.test.ts | 189 ------------------ .../host-removed-structured-chats.ts | 13 -- .../web-session-tabs-sync/store-patch.ts | 9 - .../tabs/structured-chat-close-owner.test.ts | 20 +- .../store/slices/tabs/tabs-close-actions.ts | 5 +- 10 files changed, 12 insertions(+), 368 deletions(-) delete mode 100644 src/renderer/src/app-shell/notes-delivered-by-chat-install.test.ts delete mode 100644 src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.test.ts delete mode 100644 src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.ts diff --git a/src/renderer/src/app-shell/AppBackgroundServices.tsx b/src/renderer/src/app-shell/AppBackgroundServices.tsx index caea319fd9d..ca22678decf 100644 --- a/src/renderer/src/app-shell/AppBackgroundServices.tsx +++ b/src/renderer/src/app-shell/AppBackgroundServices.tsx @@ -8,11 +8,6 @@ import { MacosTccPromptNoticeHost } from '../hooks/MacosTccPromptNoticeHost' import { useAppStore } from '../store' import { StructuredAgentSessionAttentionBridge } from '../components/native-chat/StructuredAgentSessionAttentionBridge' import { StructuredAgentSessionStatusBridge } from '../components/native-chat/StructuredAgentSessionStatusBridge' -import { installNotesDeliveredByChat } from '../lib/notes-delivered-by-chat' - -// Why at load and not in an effect: both the desktop and the web client load this with App, before -// any chat can send its saved message. -installNotesDeliveredByChat() const DashboardPopoutBridge = lazy(() => import('../components/dashboard/DashboardPopoutBridge')) diff --git a/src/renderer/src/app-shell/notes-delivered-by-chat-install.test.ts b/src/renderer/src/app-shell/notes-delivered-by-chat-install.test.ts deleted file mode 100644 index 36455b4f4ae..00000000000 --- a/src/renderer/src/app-shell/notes-delivered-by-chat-install.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { readFileSync } from 'node:fs' -import { join } from 'node:path' -import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { DiffComment } from '../../../shared/diff-comment-types' -import type { StructuredAgentSessionOutboxEntry } from '../../../shared/structured-agent-session-outbox' - -const mocks = vi.hoisted(() => ({ clearDelivered: vi.fn() })) - -const note: DiffComment = { - id: 'note-a', - worktreeId: 'wt-1', - filePath: 'README.md', - lineNumber: 3, - body: 'tighten this', - createdAt: 1, - side: 'modified' -} - -vi.mock('../store', () => ({ - useAppStore: Object.assign(() => false, { - getState: () => ({ - getDiffComments: () => [note], - clearDeliveredDiffComments: mocks.clearDelivered, - browserAnnotationsByPageId: {}, - removeDeliveredBrowserPageAnnotations: vi.fn() - }) - }) -})) -vi.mock('../components/AgentHibernationGate', () => ({ AgentHibernationGate: () => null })) -vi.mock('../components/AiVaultTabTitleSyncGate', () => ({ AiVaultTabTitleSyncGate: () => null })) -vi.mock('../components/dashboard/RetainedAgentsSyncGate', () => ({ default: () => null })) -vi.mock('../components/ports/WorkspacePortScanner', () => ({ WorkspacePortScanner: () => null })) -vi.mock('../hooks/MacosTccPromptNoticeHost', () => ({ MacosTccPromptNoticeHost: () => null })) -vi.mock('../components/native-chat/StructuredAgentSessionAttentionBridge', () => ({ - StructuredAgentSessionAttentionBridge: () => null -})) -vi.mock('../components/native-chat/StructuredAgentSessionStatusBridge', () => ({ - StructuredAgentSessionStatusBridge: () => null -})) - -const RENDERER = join(__dirname, '..') - -function carrying(keys: string[]): StructuredAgentSessionOutboxEntry { - return { - clientMessageId: 'message-1', - sessionId: 'chat-1', - body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'review notes' }] }, - previewUris: [], - state: 'rejected', - queuedAt: 1, - lastAttemptAt: null, - retryAfterUnknownSubmittedAt: null, - source: 'launch', - carriedNoteKeys: keys - } -} - -describe('clearing notes a chat sends on', () => { - beforeEach(() => { - vi.resetModules() - mocks.clearDelivered.mockReset() - }) - - // The desktop and the web client both render App, which loads its background services. - it('is installed by the background services both app entries render', async () => { - expect(readFileSync(join(RENDERER, 'main.tsx'), 'utf8')).toContain("import App from './App'") - expect(readFileSync(join(RENDERER, 'web/main.tsx'), 'utf8')).toContain("import('../App')") - expect(readFileSync(join(RENDERER, 'App.tsx'), 'utf8')).toContain( - "from './app-shell/AppBackgroundServices'" - ) - - await import('./AppBackgroundServices') - const carried = - await import('../components/native-chat/structured-agent-session-outbox-carried-notes') - const { diffCommentSendKey } = await import('../lib/notes-send-in-flight') - carried.recordStructuredAgentSessionCarriedNotes( - 'chat-1', - [carrying([diffCommentSendKey(note)])], - [], - 'spent' - ) - - expect(mocks.clearDelivered).toHaveBeenCalledWith('wt-1', [note]) - }) - - it('runs once per renderer however often it is installed', async () => { - const { installNotesDeliveredByChat } = await import('../lib/notes-delivered-by-chat') - const carried = - await import('../components/native-chat/structured-agent-session-outbox-carried-notes') - const { diffCommentSendKey } = await import('../lib/notes-send-in-flight') - installNotesDeliveredByChat() - installNotesDeliveredByChat() - - carried.recordStructuredAgentSessionCarriedNotes( - 'chat-1', - [carrying([diffCommentSendKey(note)])], - [], - 'spent' - ) - - expect(mocks.clearDelivered).toHaveBeenCalledOnce() - }) -}) diff --git a/src/renderer/src/lib/notes-delivered-by-chat.ts b/src/renderer/src/lib/notes-delivered-by-chat.ts index 43839295532..15872b31e19 100644 --- a/src/renderer/src/lib/notes-delivered-by-chat.ts +++ b/src/renderer/src/lib/notes-delivered-by-chat.ts @@ -6,15 +6,10 @@ import { noteSendKeyOwner } from './notes-send-in-flight' -let uninstall: (() => void) | null = null - /** A new chat's message sent on, by its own start, a Retry or a re-check, clears the notes it - * carries from their shelf as a delivered send does, including after a reload. Once per renderer. */ -export function installNotesDeliveredByChat(): void { - if (uninstall) { - return - } - uninstall = subscribeToStructuredAgentSessionCarriedNotesSpent((keys) => { + * carries from their shelf as a delivered send does, including after a reload. */ +export function installNotesDeliveredByChat(): () => void { + return subscribeToStructuredAgentSessionCarriedNotesSpent((keys) => { const sent = new Set(keys) const state = useAppStore.getState() const worktreeIds = new Set() @@ -45,8 +40,3 @@ export function installNotesDeliveredByChat(): void { } }) } - -import.meta.hot?.dispose(() => { - uninstall?.() - uninstall = null -}) diff --git a/src/renderer/src/main.tsx b/src/renderer/src/main.tsx index 36239d52516..b70b662dc8a 100644 --- a/src/renderer/src/main.tsx +++ b/src/renderer/src/main.tsx @@ -24,11 +24,13 @@ import { getOrCreateRendererRoot } from './lib/react-renderer-root' import { primeTerminalWebglAddon } from './lib/pane-manager/pane-webgl-renderer' import { SkillWarningPreviewLauncher } from './components/skills/SkillWarningPreviewLauncher' import { installBrowserClientPageRenderer } from './components/browser-pane/browser-client-page-renderer-installation' +import { installNotesDeliveredByChat } from './lib/notes-delivered-by-chat' recordRendererCrashBreadcrumb('renderer_bootstrap_started', { dev: import.meta.env.DEV }) installRendererCrashDiagnostics() installTypingLatencyDiagnostic() installAutomationHostDiagnostic() +installNotesDeliveredByChat() if ( import.meta.env.DEV && diff --git a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts index c15105b80f0..678b915d49c 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-browser.ts @@ -21,7 +21,6 @@ import { } from './state-equality-files' import { shouldRetainStructuredAgentSessionLaunchTab } from '@/lib/structured-agent-session-launch-registry' import { executionHostIdForSessionTabsOwner } from '../local-structured-session-owner' -import { noteHostRemovedStructuredChat } from './host-removed-structured-chats' export function prepareWebSessionTabsSnapshotBrowser( base: ReturnType @@ -185,18 +184,11 @@ export function prepareWebSessionTabsSnapshotBrowser( // A matching host row is authoritative; retaining the provisional tab beside its mirror // would briefly render two panes before lifecycle publication is recorded. // A host that cannot list its chats is no evidence this one closed. - if (publishedAgentSessionIds.has(tab.entityId)) { - return false - } - if ( - !agentSessionsAffirmed || - shouldRetainStructuredAgentSessionLaunchTab(worktreeId, tab.entityId) - ) { - return true - } - // Listed before, affirmed absent now: the chat itself is gone, not a launch still starting. - noteHostRemovedStructuredChat(tab.entityId) - return false + return ( + !publishedAgentSessionIds.has(tab.entityId) && + (!agentSessionsAffirmed || + shouldRetainStructuredAgentSessionLaunchTab(worktreeId, tab.entityId)) + ) } if (tab.contentType === 'browser') { return ( diff --git a/src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.test.ts b/src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.test.ts deleted file mode 100644 index f3b6ef3a1df..00000000000 --- a/src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.test.ts +++ /dev/null @@ -1,189 +0,0 @@ -// @vitest-environment happy-dom -import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { Tab } from '../../../../shared/tab-types' -import type { WebSessionTabsSyncState } from './state' - -const store = vi.hoisted(() => { - const state: Record = {} - return { state } -}) - -vi.mock('@/store', () => ({ - useAppStore: { - getState: () => ({ ...store.state, scheduleAgentStatusFreshness: () => undefined }), - setState: (update: (state: Record) => Record) => { - store.state = { ...store.state, ...update(store.state) } - } - } -})) - -vi.mock('@/lib/launch-structured-agent-session', () => { - class StructuredAgentSessionCreateRefusalError extends Error {} - return { - createStructuredAgentSessionLaunchIntent: (worktreeId: string) => ({ - worktreeId, - sessionId: 'launch-1', - executionHostId: 'local', - target: { kind: 'local' }, - agent: 'codex', - params: { - envelope: { - sessionId: 'launch-1', - clientOperationId: 'operation-launch-1', - expectedRuntimeFence: null, - payloadFingerprint: 'fingerprint-launch-1' - }, - worktree: `id:${worktreeId}`, - agent: 'codex' - } - }), - retryStructuredAgentSessionLaunchIntent: (intent: unknown) => intent, - restoreStructuredAgentSessionLaunchIntent: vi.fn(), - abandonStructuredAgentSessionLaunchIntent: vi.fn(), - launchStructuredAgentSession: () => - Promise.reject(new StructuredAgentSessionCreateRefusalError('unsupported')), - StructuredAgentSessionCreateRefusalError - } -}) - -import { applyWebSessionTabsSnapshot } from '../web-session-tabs-sync' -import { applyWebSessionTabsStorePatch } from './store-patch' -import { - makeSnapshot, - makeState, - resetWebSessionTabsSyncTestState, - ENV, - NOW, - WT -} from '../web-session-tabs-sync-test-harness' -import { - appendStructuredAgentSessionOutboxMessage, - readOutbox -} from '@/components/native-chat/structured-agent-session-outbox-storage' -import { resetStructuredAgentSessionCarriedNotesForTests } from '@/components/native-chat/structured-agent-session-outbox-carried-notes' -import { isNoteInFlight } from '@/lib/notes-send-in-flight' -import { resetStructuredAgentLaunchRegistryForTests } from '@/lib/structured-agent-session-launch-registry' -import { resetStructuredAgentLaunchPersistenceForTests } from '@/lib/structured-agent-session-launch-persistence' -import { startStructuredAgentLaunch } from '@/lib/structured-agent-session-launch' -import { takeHostRemovedStructuredChats } from './host-removed-structured-chats' - -const NOTE_KEY = 'note-a' - -function chatTab(sessionId: string, id = `tab-${sessionId}`): Tab { - return { - id, - entityId: sessionId, - contentType: 'agent-session', - agentSessionAgent: 'codex', - worktreeId: WT, - groupId: 'group-1', - label: 'Codex', - customLabel: null, - color: null, - createdAt: 1, - sortOrder: 0 - } -} - -function windowWith(tabs: Tab[]): WebSessionTabsSyncState { - return makeState({ - unifiedTabsByWorktree: { [WT]: tabs }, - groupsByWorktree: { - [WT]: [ - { - id: 'group-1', - worktreeId: WT, - tabOrder: tabs.map((tab) => tab.id), - activeTabId: tabs[0]?.id ?? null - } - ] - }, - activeGroupIdByWorktree: { [WT]: 'group-1' } - }) -} - -/** A chat whose notes message the host refused: it waits on its Retry, carrying the notes. */ -function refusedNotesMessage(sessionId: string): void { - appendStructuredAgentSessionOutboxMessage(sessionId, 'review notes', [], 'launch', [NOTE_KEY]) -} - -/** The host's next list of chats, as this window's sync commits it. */ -function hostLists(sessions: string[]): void { - applyWebSessionTabsStorePatch( - (state) => - applyWebSessionTabsSnapshot( - state, - makeSnapshot( - sessions.map((sessionId) => ({ - type: 'agent-session' as const, - id: `agent-session:${sessionId}`, - sessionId, - agent: 'codex' as const, - title: 'Codex', - isActive: false - })), - { snapshotVersion: 2 } - ), - ENV, - NOW, - { contentScope: 'agent-session', preserveLocalLayout: true, terminalPtyMode: 'local' } - ), - { frames: [] } - ) -} - -describe('a chat the host stops listing', () => { - beforeEach(() => { - resetWebSessionTabsSyncTestState() - localStorage.clear() - resetStructuredAgentSessionCarriedNotesForTests() - resetStructuredAgentLaunchRegistryForTests() - resetStructuredAgentLaunchPersistenceForTests() - takeHostRemovedStructuredChats() - }) - - it('takes its queued messages with it, so the notes they carried come back', () => { - store.state = { ...windowWith([chatTab('chat-1')]) } - refusedNotesMessage('chat-1') - expect(isNoteInFlight(NOTE_KEY)).toBe(true) - - hostLists([]) - - expect(readOutbox('chat-1')).toEqual([]) - expect(isNoteInFlight(NOTE_KEY)).toBe(false) - }) - - it('keeps a chat the host still lists, under another local tab id', () => { - store.state = { ...windowWith([chatTab('chat-1', 'chat-1:history-1')]) } - refusedNotesMessage('chat-1') - - hostLists(['chat-1']) - - expect(readOutbox('chat-1')).toHaveLength(1) - expect(isNoteInFlight(NOTE_KEY)).toBe(true) - }) - - it("keeps a failed launch's messages for its Retry: the host never listed it", async () => { - const failed = startFailedLaunch() - await settle() - store.state = { ...windowWith([chatTab(failed)]) } - - hostLists([]) - - expect(readOutbox(failed)).toHaveLength(1) - expect(isNoteInFlight(NOTE_KEY)).toBe(true) - }) -}) - -function startFailedLaunch(): string { - return startStructuredAgentLaunch(WT, 'codex', { - prompt: 'review notes', - carriedNoteKeys: [NOTE_KEY] - }).sessionId -} - -async function settle(): Promise { - for (let i = 0; i < 20; i += 1) { - await Promise.resolve() - } -} diff --git a/src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.ts b/src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.ts deleted file mode 100644 index e3ae45fc06c..00000000000 --- a/src/renderer/src/runtime/web-session-tabs-sync/host-removed-structured-chats.ts +++ /dev/null @@ -1,13 +0,0 @@ -// Why: a chat the host stopped listing is gone, wherever it was closed (the phone, another window, -// while Orca was off). Noted while a sync patch is prepared, acted on only once that patch lands. -const removedSessionIds = new Set() - -export function noteHostRemovedStructuredChat(sessionId: string): void { - removedSessionIds.add(sessionId) -} - -export function takeHostRemovedStructuredChats(): string[] { - const sessionIds = [...removedSessionIds] - removedSessionIds.clear() - return sessionIds -} diff --git a/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts b/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts index 1f45178027c..db2758d4bbe 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/store-patch.ts @@ -17,8 +17,6 @@ import { type HostSessionMirrorPatchVerdict, type HostSessionMirrorSettle } from './mirror-settle' -import { takeHostRemovedStructuredChats } from './host-removed-structured-chats' -import { discardStructuredAgentSessionLaunchOutbox } from '@/components/native-chat/structured-agent-session-outbox-storage' /** Commit a reconciliation patch and return a receipt for its host evidence. */ export function applyWebSessionTabsStorePatch( @@ -151,13 +149,10 @@ export function applyWebSessionTabsStorePatch( return patch } - // Only what this patch's own preparation notes. - takeHostRemovedStructuredChats() try { useAppStore.setState(runStorePatch) } catch (error) { if (!patchCommitted) { - takeHostRemovedStructuredChats() throw error } console.warn('[web-session-tabs-sync] a store subscriber failed after the patch landed:', error) @@ -165,10 +160,6 @@ export function applyWebSessionTabsStorePatch( const settleHostMirror = createHostSessionMirrorSettle(hostMirrorVerdict) try { - // A chat gone from its host takes its queued messages, and the notes they held, with it. - for (const sessionId of takeHostRemovedStructuredChats()) { - discardStructuredAgentSessionLaunchOutbox(sessionId) - } if (mirroredAgentStatusChanged) { useAppStore.getState().scheduleAgentStatusFreshness() } diff --git a/src/renderer/src/store/slices/tabs/structured-chat-close-owner.test.ts b/src/renderer/src/store/slices/tabs/structured-chat-close-owner.test.ts index 54df71490c5..0c9e658cac1 100644 --- a/src/renderer/src/store/slices/tabs/structured-chat-close-owner.test.ts +++ b/src/renderer/src/store/slices/tabs/structured-chat-close-owner.test.ts @@ -1,9 +1,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { Tab } from '../../../../../shared/tab-types' import { createTestStore, makeWorktree, seedStore } from '../store-test-helpers' -import type * as OutboxStorageModule from '@/components/native-chat/structured-agent-session-outbox-storage' -const mocks = vi.hoisted(() => ({ beginClose: vi.fn(), discardOutbox: vi.fn() })) +const mocks = vi.hoisted(() => ({ beginClose: vi.fn() })) vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn(), warning: vi.fn() } @@ -11,13 +10,6 @@ vi.mock('sonner', () => ({ vi.mock('@/runtime/structured-agent-session-tab-retirement', () => ({ beginStructuredAgentSessionTabClose: mocks.beginClose })) -vi.mock( - '@/components/native-chat/structured-agent-session-outbox-storage', - async (importOriginal) => ({ - ...(await importOriginal()), - discardStructuredAgentSessionLaunchOutbox: mocks.discardOutbox - }) -) // `repoId::path` names both checkouts: this machine's and the paired server's. const WORKTREE = 'repo-1::/work/app' @@ -66,7 +58,6 @@ function storeWith(tab: Tab): ReturnType { beforeEach(() => { mocks.beginClose.mockReset() - mocks.discardOutbox.mockReset() }) describe('closing a structured chat from outside its workspace', () => { @@ -91,13 +82,4 @@ describe('closing a structured chat from outside its workspace', () => { expect(mocks.beginClose).not.toHaveBeenCalled() expect(store.getState().unifiedTabsByWorktree[WORKTREE] ?? []).toEqual([]) }) - - // Its queued messages go with it, as for any close, so notes they carried come back. - it('throws away the queued messages of a chat it can name no host for', () => { - const store = storeWith(chatTab()) - - store.getState().closeUnifiedTab('agent-session:chat-1') - - expect(mocks.discardOutbox).toHaveBeenCalledWith('chat-1') - }) }) diff --git a/src/renderer/src/store/slices/tabs/tabs-close-actions.ts b/src/renderer/src/store/slices/tabs/tabs-close-actions.ts index a2edefb0f95..f4cd23a01d7 100644 --- a/src/renderer/src/store/slices/tabs/tabs-close-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-close-actions.ts @@ -9,7 +9,6 @@ import { } from '../tab-group-state' import { buildActiveSurfacePatch } from './tabs-surface' import { beginStructuredAgentSessionTabClose } from '@/runtime/structured-agent-session-tab-retirement' -import { discardStructuredAgentSessionLaunchOutbox } from '@/components/native-chat/structured-agent-session-outbox-storage' import { hasStructuredAgentSessionLaunchCancellationTombstone, shouldRetainStructuredAgentSessionLaunchTab @@ -70,10 +69,8 @@ export function createTabsCloseActions( provisional }) } else { - // Closing still removes the tab; no host can be named to stop its chat on. Its queued - // messages go with it, as for any close, so notes they carried return to their shelf. + // Closing still removes the tab; no host can be named to stop its chat on. console.warn('[structured-agent-session] close found no owning host', tab.entityId) - discardStructuredAgentSessionLaunchOutbox(tab.entityId) } get().clearNativeChatLaunchDraft(structuredAgentSessionTabId(tab.entityId)) }