From 920a06adcf479f3fd5abda09e01876ca06c06bd2 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sat, 5 Sep 2026 19:34:21 -0700 Subject: [PATCH] fix(native-chat): stop a reveal's own inventory refresh discarding its republished tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual QA: the host answered reveal with ok:true and republished the tab, and the chat still did not reopen — only a renderer reload brought it back. The renderer publishes under one epoch string for its whole lifetime, and a frame recorded under a different lineage retires that epoch permanently with nothing to un-retire it. The Resume click asks for an inventory first, and a worktree the host holds no entry for answers with the none/v0 sentinel; the structured path recorded it, retiring the renderer's own epoch, so the tab the reveal published a moment later was dropped. A reload minted a new epoch, which is why reloading appeared to fix it. A frame that carries no publication is not a later publication to fence against. Treat the sentinel and a removal frame as a cursor reset, the way the mainstream session-tabs path already clears its tracking — its comment names this exact hazard: recording that sentinel would retire the host epoch and reject the next live frame. Pre-existing, and it swallows an ordinary new-tab launch on an empty worktree too; the reveal is what turned a silent invisibility into a visible failure. --- ...ructured-session-reveal-visibility.test.ts | 153 ++++++++++++++++++ .../snapshot-apply.ts | 13 ++ 2 files changed, 166 insertions(+) create mode 100644 src/renderer/src/runtime/local-structured-session-reveal-visibility.test.ts diff --git a/src/renderer/src/runtime/local-structured-session-reveal-visibility.test.ts b/src/renderer/src/runtime/local-structured-session-reveal-visibility.test.ts new file mode 100644 index 00000000000..a814972dfe5 --- /dev/null +++ b/src/renderer/src/runtime/local-structured-session-reveal-visibility.test.ts @@ -0,0 +1,153 @@ +// @vitest-environment happy-dom + +/** + * Reopening a closed chat has to survive the frames the reopen itself provokes. + * + * The renderer publishes under one epoch string for its whole lifetime, so a frame recorded under + * a different lineage retires that epoch permanently — and the click that reopens a chat asks the + * host for an inventory first, which answers `none`/v0 for a worktree it holds no entry for. That + * answer used to be recorded, retiring the renderer's own epoch and dropping the republished tab + * that arrived moments later. The chat only reappeared after a reload minted a new epoch. + */ + +import { afterEach, describe, expect, it } from 'vitest' +import { UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH } from '../../../shared/runtime-types' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import type { WorktreeRuntimeOwnerState } from '../lib/worktree-runtime-owner' +import { + applyLocalStructuredSessionTabSnapshots, + resetLocalStructuredSessionVersionForTests +} from './local-structured-session-tabs-sync' +import type { WebSessionTabsSyncState } from './web-session-tabs-sync' +import { resetWebSessionTabsSnapshotFreshnessForTests } from './web-session-tabs-sync' + +const WORKTREE = 'repo-1::/tmp/wt-reveal' +const HOST_TAB_ID = 'agent-session:codex-reveal-1' +// The projection renames a host tab id into the renderer's own namespace. +const SESSION_TAB = 'structured-agent-session-codex-reveal-1' +// One string for the renderer's whole lifetime, which is exactly why retiring it is unrecoverable. +const RENDERER_EPOCH = 'renderer:11111111-2222-3333-4444-555555555555' + +type SyncState = WebSessionTabsSyncState & WorktreeRuntimeOwnerState + +afterEach(() => { + resetLocalStructuredSessionVersionForTests() + resetWebSessionTabsSnapshotFreshnessForTests() +}) + +function baseState(): SyncState { + return { + activeBrowserTabId: null, + activeBrowserTabIdByWorktree: {}, + activeFileId: null, + activeFileIdByWorktree: {}, + activeGroupIdByWorktree: {}, + activeTabId: null, + activeTabIdByWorktree: {}, + activeTabType: 'terminal', + activeTabTypeByWorktree: {}, + activeWorktreeId: WORKTREE, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, + browserCertificateFailuresByPageId: {}, + browserPagesByWorkspace: {}, + browserTabsByWorktree: {}, + groupsByWorktree: {}, + layoutByWorktree: {}, + openFiles: [], + ptyIdsByTabId: {}, + remoteBrowserPageHandlesByPageId: {}, + tabBarOrderByWorktree: {}, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + unifiedTabsByWorktree: {}, + unreadTerminalTabs: {}, + sortEpoch: 0, + // Required: without a catalog entry the trailing cleanup deletes the cursor and hides the bug. + worktreesByRepo: { 'repo-1': [{ id: WORKTREE, path: '/tmp/wt-reveal' }] } + } as unknown as SyncState +} + +function chatFrame(epoch: string, version: number): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE, + publicationEpoch: epoch, + snapshotVersion: version, + activeGroupId: `headless-terminals:${WORKTREE}`, + activeTabId: HOST_TAB_ID, + activeTabType: 'agent-session', + tabs: [ + { + type: 'agent-session', + id: HOST_TAB_ID, + title: 'Codex Chat', + sessionId: 'codex-reveal-1', + agent: 'codex', + isActive: true + } + ] + } as unknown as RuntimeMobileSessionTabsResult +} + +function emptyFrame(epoch: string, version: number): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE, + publicationEpoch: epoch, + snapshotVersion: version, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [] + } as unknown as RuntimeMobileSessionTabsResult +} + +function apply(state: SyncState, snapshot: RuntimeMobileSessionTabsResult): SyncState { + return applyLocalStructuredSessionTabSnapshots(state, [snapshot]) +} + +function chatTabIds(state: SyncState): string[] { + return (state.unifiedTabsByWorktree[WORKTREE] ?? []) + .filter((tab) => tab.contentType === 'agent-session') + .map((tab) => tab.id) +} + +describe('a revealed chat survives the frames the reveal provokes', () => { + it('reopens after the click asks an unpublished worktree for its inventory', () => { + let state = apply(baseState(), chatFrame(RENDERER_EPOCH, 120)) + expect(chatTabIds(state)).toHaveLength(1) + + state = apply(state, emptyFrame(RENDERER_EPOCH, 121)) // the user closes the chat + expect(chatTabIds(state)).toEqual([]) + + // The Resume click's own `session.tabs.list`: the host holds no entry, so it answers the + // sentinel. Recording it retired the renderer's epoch and poisoned the reveal that follows. + state = apply(state, emptyFrame(UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH, 0)) + + state = apply(state, chatFrame(RENDERER_EPOCH, 122)) // reveal republishes + + expect(chatTabIds(state)).toEqual([SESSION_TAB]) + }) + + it('reopens after the host pruned and rebuilt its entry for the worktree', () => { + // A rebuilt host entry restarts its own counter, so the republished frame can carry a version + // below the cursor the renderer recorded. Fencing on that alone discards a live publication. + let state = apply(baseState(), chatFrame(RENDERER_EPOCH, 120)) + state = apply(state, emptyFrame(RENDERER_EPOCH, 121)) + + state = apply(state, { ...emptyFrame('removed:abc', 0), removed: true } as never) + state = apply(state, chatFrame(RENDERER_EPOCH, 9)) + + expect(chatTabIds(state)).toEqual([SESSION_TAB]) + }) + + it('still ignores a genuinely superseded republication', () => { + // The fences exist for a reason: without an intervening non-publication frame, an older + // version under the same lineage must still lose. + let state = apply(baseState(), chatFrame(RENDERER_EPOCH, 120)) + state = apply(state, emptyFrame(RENDERER_EPOCH, 121)) + + state = apply(state, chatFrame(RENDERER_EPOCH, 9)) + + expect(chatTabIds(state)).toEqual([]) + }) +}) diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts index fc254de62dc..b4ddd770323 100644 --- a/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts @@ -22,6 +22,7 @@ import { supersedeLocalStructuredSessionGeneration } from './inventory-generation-fence' import { projectLocalStructuredSessionTabs } from './snapshot-projection' +import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' export const LOCAL_STRUCTURED_SESSION_OWNER = 'local-structured-session' @@ -73,6 +74,18 @@ export function applyLocalStructuredSessionTabSnapshots< if (getExecutionHostIdForWorktree(next, snapshot.worktree) !== 'local') { continue } + // A frame that carries no publication is not a later publication to fence against. Recording + // its epoch retires the renderer's own — which is a module constant for the process lifetime, + // and nothing un-retires it — so every republication afterwards is dropped until a reload + // mints a new one. That is what made a revealed chat invisible: the click's own inventory + // refresh reads `none`/v0 for a worktree the host holds no entry for, and poisons the reveal + // that follows it. The mainstream session-tabs path clears its tracking here for the same + // reason; this one recorded the sentinel instead. + if (snapshot.removed === true || !hostSnapshotAffirmsWorktreeContents(snapshot)) { + localStructuredSessionVersionByWorktree.delete(snapshot.worktree) + localStructuredSessionEpochHistoryByWorktree.delete(snapshot.worktree) + continue + } const prior = localStructuredSessionVersionByWorktree.get(snapshot.worktree) const sharesLineage = Boolean( prior && sameSessionTabsPublicationLineage(prior.publicationEpoch, snapshot.publicationEpoch)