refactor(mobile-sync): share the published-active-group rule with the structured projection

Review follow-up. `resolvePublishedActiveGroupId` was a second copy of the rule
already in `projectLocalStructuredSessionTabs`; extract it once and call it from
both, so the structured projection cannot drift from the publisher.

Ablating the shared rule left the structured projection's own tests green, so
pin it there too: a snapshot whose active group holds no visible structured tab
must not keep pointing at it, and must fall to null when nothing survives.

Also repairs a cross-file comment reference that dangled after the relocation.
This commit is contained in:
Brennan Benson
2026-08-29 20:12:22 -07:00
committed by Merge Sim
parent bc195b63ec
commit 1fa70bf34d
5 changed files with 89 additions and 24 deletions
@@ -208,6 +208,65 @@ describe('local structured session tab projection', () => {
tabGroupLayout: undefined,
tabs: [expect.objectContaining({ type: 'agent-session', agent: 'codex' })]
})
// The surviving group is still the active one, so the pointer passes through untouched.
expect(projectLocalStructuredSessionTabs(snapshot).activeGroupId).toBe('structured-group')
})
// Same invariant as the mobile snapshot publisher: this projection drops every group with no
// visible structured tab, so an active-group pointer at a dropped one would name a group the
// caller never receives.
it('does not keep an active-group pointer at a group it dropped', () => {
const base = {
worktree: 'workspace-1',
publicationEpoch: 'epoch-1',
snapshotVersion: 1,
activeGroupId: 'terminal-group',
activeTabId: 'terminal-1',
activeTabType: 'terminal',
tabGroups: [
{ id: 'terminal-group', activeTabId: 'terminal-1', tabOrder: ['terminal-1'] },
{
id: 'structured-group',
activeTabId: 'agent-session:codex-1',
tabOrder: ['agent-session:codex-1']
}
],
tabs: [
{
type: 'terminal',
id: 'terminal-1',
parentTabId: 'terminal-1',
leafId: 'leaf-1',
title: 'Terminal',
status: 'ready',
terminal: 'term-1',
ptyId: 'pty-1',
isActive: true
},
{
type: 'agent-session',
id: 'agent-session:codex-1',
title: 'Codex Chat',
sessionId: 'codex-1',
agent: 'codex',
isActive: false
}
]
} satisfies RuntimeMobileSessionTabsResult
const projected = projectLocalStructuredSessionTabs(base)
// The presence half: the structured group does survive, so an empty projection fails here.
expect(projected.tabGroups?.map((group) => group.id)).toEqual(['structured-group'])
expect(projected.activeGroupId).toBe('structured-group')
// And with nothing left to point at, the pointer goes to null rather than a dropped group.
const structuredDropped = projectLocalStructuredSessionTabs({
...base,
tabs: base.tabs.filter((tab) => tab.type !== 'agent-session')
} satisfies RuntimeMobileSessionTabsResult)
expect(structuredDropped.tabGroups).toEqual([])
expect(structuredDropped.activeGroupId).toBeNull()
})
it('preserves the exact local split through apply, persistence, and hydration', () => {
@@ -10,6 +10,7 @@ import {
decideWebSessionTabsSnapshot
} from './web-session-tabs-sync'
import type { WebSessionTabsSyncState } from './web-session-tabs-sync'
import { resolvePublishedActiveGroupId } from './mobile-session-published-active-group'
export const LOCAL_STRUCTURED_SESSION_OWNER = 'local-structured-session'
let localStructuredSessionTabsRestorePromise: Promise<void> | null = null
@@ -42,11 +43,7 @@ export function projectLocalStructuredSessionTabs(
activeTabId: visibleIds.has(snapshot.activeTabId ?? '') ? snapshot.activeTabId : null,
activeTabType:
snapshot.activeTabId && visibleIds.has(snapshot.activeTabId) ? snapshot.activeTabType : null,
activeGroupId:
snapshot.activeGroupId &&
projectedTabGroups?.some((group) => group.id === snapshot.activeGroupId)
? snapshot.activeGroupId
: (projectedTabGroups?.[0]?.id ?? null),
activeGroupId: resolvePublishedActiveGroupId(snapshot.activeGroupId, projectedTabGroups),
tabs: snapshot.tabs.filter((tab) => visibleIds.has(tab.id)),
tabGroups: projectedTabGroups,
// Why: group membership locates chats; the renderer's split tree remains locally authoritative.
@@ -0,0 +1,17 @@
import type { RuntimeMobileSessionTabGroup } from '../../../shared/runtime-session-contracts'
/**
* The active-group pointer a mobile snapshot is allowed to publish.
*
* Why: a group whose every tab was held back never reaches the client, so naming it would point at
* something the client was never sent. Falls back to the first group that did publish, then null.
*/
export function resolvePublishedActiveGroupId(
activeGroupId: string | null,
publishedGroups: readonly RuntimeMobileSessionTabGroup[] | undefined
): string | null {
if (activeGroupId && publishedGroups?.some((group) => group.id === activeGroupId)) {
return activeGroupId
}
return publishedGroups?.[0]?.id ?? null
}
@@ -35,8 +35,9 @@ function danglingGroupRefs(
}
describe('buildMobileSessionTabSnapshots published group references', () => {
// Why the same claim is driven again through tab groups: the test above takes the legacy nav
// order, where the only thing that has to hold the document back is the per-tab guard. With
// Why groups get their own fixture: 'omits a browser tab located by a workspace document from
// mobile snapshots' (in sync-runtime-graph-editor-diff-tabs.test.ts) drives the same document
// through the legacy nav order, where the per-tab guard is the only thing holding it back. With
// groups the projection runs first and publishes group metadata of its own, so a document reaching
// it leaves a group whose tabOrder and activeTabId name a tab the phone is never sent.
it('keeps a workspace document out of published tab groups as well as the tab list', () => {
+8 -17
View File
@@ -29,6 +29,7 @@ import type {
import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/stable-pane-id'
import { isWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id'
import { isClaudeManagementTitle } from '../../../shared/agent-detection'
import { resolvePublishedActiveGroupId } from './mobile-session-published-active-group'
import { parseWorkspaceKey } from '../../../shared/workspace-scope'
import type { Tab, TabGroup, TabGroupLayoutNode } from '../../../shared/tab-types'
import type {
@@ -1420,18 +1421,18 @@ export function buildMobileSessionTabSnapshots(
fallbackEditorTabs,
active?.id ?? null
)
const tabGroupLayout =
tabGroups && tabGroups.length > 0
? pruneTabGroupLayout(inputs.tabGroupLayout, new Set(tabGroups.map((group) => group.id)))
: groupProjection.tabGroupLayout
const publishedGroups = tabGroups && tabGroups.length > 0 ? tabGroups : undefined
const tabGroupLayout = publishedGroups
? pruneTabGroupLayout(
inputs.tabGroupLayout,
new Set(publishedGroups.map((group) => group.id))
)
: groupProjection.tabGroupLayout
const content = {
// Why not the desktop's own active group: a group whose every tab was held back never reaches
// the client, so naming it here points at something the client was never sent (STA-5724).
activeGroupId: resolvePublishedActiveGroupId(activeGroupId, publishedGroups),
activeTabId: active?.id ?? null,
activeTabType: active?.type ?? null,
...(tabGroups && tabGroups.length > 0 ? { tabGroups } : {}),
...(publishedGroups ? { tabGroups: publishedGroups } : {}),
...(tabGroupLayout ? { tabGroupLayout } : {}),
tabs
}
@@ -1502,16 +1503,6 @@ function applyUnifiedEditorTabIdsToLegacyOrder(
})
}
function resolvePublishedActiveGroupId(
activeGroupId: string | null,
publishedGroups: readonly RuntimeMobileSessionTabGroup[] | undefined
): string | null {
if (activeGroupId && publishedGroups?.some((group) => group.id === activeGroupId)) {
return activeGroupId
}
return publishedGroups?.[0]?.id ?? null
}
function appendFallbackEditorTabsToGroups(
tabGroups: RuntimeMobileSessionTabGroup[] | undefined,
sourceGroups: readonly TabGroup[],