From 323c7ae8801f8e65122fa41212536aeffd64a584 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:40:42 -0700 Subject: [PATCH] fix(terminal): collapse identity group in the title churn signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the ingest-time title rewrite from #16373 with a non-destructive fix at the actual cause. The churn suppressor `isDecorativeAgentTitleFrameChange` keyed on the literal label, so `working:OMP` and `working:Pi` compared unequal and every alternating frame from a wrapped harness committed a store patch. #16373 made the labels agree by rewriting the stored title to the tab's launch owner — but `runtimePaneTitlesByTabId` is also the Windows Shift+Enter byte-encoding input, so normalizing at ingest destroyed evidence other consumers read (fixed separately in #16376). Collapse the identity group inside the signature instead. Which member of a group a frame names is decoration, exactly like the spinner glyph the signature already strips, so frames compare equal without touching what is stored. Suppression now changes only WHETHER a frame commits, never WHAT it says. Also fixes the flap under a multiplexer (#8032): the collapse runs over wrapper segments, so "zsh | ⠋ Pi" and "zsh | ⠙ OMP" compare equal, which the anchored owner-relabel in #16373 never matched. Reverts the store changes from #16373 and drops the helper it added. Co-authored-by: Seongho.Bak <49228032+psh4607@users.noreply.github.com> --- ...p-pi-alternating-title-frame-churn.test.ts | 132 ++++++++++++------ .../store/slices/terminal-tab-title-batch.ts | 6 +- .../terminals/terminal-tab-presentation.ts | 22 +-- .../agent-decorative-title-signature.ts | 8 +- src/shared/agent-title-owner.ts | 47 +++---- 5 files changed, 112 insertions(+), 103 deletions(-) diff --git a/src/renderer/src/store/slices/repro-omp-pi-alternating-title-frame-churn.test.ts b/src/renderer/src/store/slices/repro-omp-pi-alternating-title-frame-churn.test.ts index 0fdfb3fb157..6cbc3565560 100644 --- a/src/renderer/src/store/slices/repro-omp-pi-alternating-title-frame-churn.test.ts +++ b/src/renderer/src/store/slices/repro-omp-pi-alternating-title-frame-churn.test.ts @@ -7,38 +7,31 @@ * same pane, and both are "correct" from their own vantage point: * * A) main's synthetic title spinner. `driveSyntheticTitleFromHook` starts a - * shared 80ms interval (src/main/index.ts:1947 SPINNER_FRAMES, :1948 - * SPINNER_INTERVAL_MS) that injects `\x1b]0; \x07` - * (:2170, :2205). For an OMP-owned pane the profile is `omp`, so every tick - * asserts " OMP". + * shared 80ms interval (src/main/index.ts SPINNER_INTERVAL_MS) that injects + * `\x1b]0; \x07`. For an OMP-owned pane the + * profile is `omp`, so every tick asserts " OMP". * B) the OMP process itself, which is Pi underneath and emits Pi's own frames. - * "⠋ Pi" passes through untouched, and legacy "π …" frames are collapsed to - * a HARDCODED "Pi" by `normalizeTerminalTitle` - * (src/shared/agent-title-status.ts:135-144) — that collapse is - * identity-blind, so an OMP-owned pane still yields the literal "Pi". * * `pi` and `omp` share `titleIdentityGroup: 'pi-compatible'` - * (src/shared/synthetic-agent-title.ts:45-56), so these are the SAME identity as - * far as ownership is concerned — but neither writer is normalized to the tab's - * launch owner before it reaches the store. + * (src/shared/synthetic-agent-title.ts), so to a pane these are the SAME agent. * - * The churn suppressor that normally absorbs spinner noise is defeated here. + * The churn suppressor that normally absorbs spinner noise was defeated here. * `isDecorativeAgentTitleFrameChange` keys on `status:textWithoutSpinner` - * (src/shared/agent-decorative-title-signature.ts:17): "⠋ OMP" -> `working:OMP`, - * "⠙ Pi" -> `working:Pi`. The signatures differ, so every alternating frame is - * classified as a MEANINGFUL change and commits — through - * `applyTerminalTabTitleUpdates` - * (src/renderer/src/store/slices/terminal-tab-title-batch.ts:183) for `tab.title` - * (the string `resolveTerminalTabTitle` renders in the tab bar) and through - * `setRuntimePaneTitle` - * (src/renderer/src/store/terminals/terminal-tab-presentation.ts:145-172) for the - * pane slot. At 80ms that is ~12 committed patches per second, per working OMP - * tab, with a visibly flickering label. + * (src/shared/agent-decorative-title-signature.ts): "⠋ OMP" -> `working:OMP`, + * "⠙ Pi" -> `working:Pi`. Different signatures, so every alternating frame was + * classified as a MEANINGFUL change and committed — through + * `applyTerminalTabTitleUpdates` for `tab.title` and through + * `setRuntimePaneTitle` for the pane slot. At 80ms that is ~12 committed + * patches per second per working OMP tab, with a visibly flickering label. * - * The oracle is commit COUNT, not exact text: the status never leaves 'working' - * across the sequence, so a correctly owner-pinned title settles after the first - * frame and every later frame is decoration. Both tests drive the REAL store - * actions over the REAL frame text main emits. + * The fix collapses the identity GROUP inside the decorative signature, so a + * frame naming either member compares equal. Deliberately NOT a rewrite of the + * stored title: `runtimePaneTitlesByTabId` is also the Windows Shift+Enter + * byte-encoding input (keyboard-handlers.ts -> terminal-windows-shift-enter.ts), + * so normalizing titles at ingest destroys evidence other consumers read. + * Suppression changes only WHETHER a frame commits, never WHAT it says. + * + * The oracle is commit COUNT plus byte-identity of what does land. */ import { describe, expect, it, vi } from 'vitest' @@ -159,29 +152,76 @@ describe('OMP-owned tab title across interleaved OMP/Pi spinner frames', () => { ).toBe('OMP') }) - // Why: relabeling is for frames naming a DIFFERENT group member. A frame that already names the - // owner carries its own status wording, so restating bare "OMP" as "OMP ready" would change a - // tab that never flapped — and the same guard keeps a plain Pi-owned tab byte-identical. - it.each([ - ['omp', 'OMP'], - ['pi', 'Pi'] - ] as const)('leaves a %s-owned tab’s own identity frames untouched', (launchAgent, label) => { + // Why: the fix must never rewrite what a frame says — `runtimePaneTitlesByTabId` feeds the + // Windows Shift+Enter encoding, so a normalized title there silently changes keyboard bytes. + it('stores every committed frame byte-for-byte, never a relabeled form', () => { const store = createTestStore() - seedStore(store, { - worktreesByRepo: { repo1: [makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt-omp' })] }, - tabsByWorktree: { - [WT]: [makeTab({ id: TAB_ID, worktreeId: WT, title: 'Terminal 1', launchAgent })] - }, - unifiedTabsByWorktree: { - [WT]: [makeUnifiedTab({ id: TAB_ID, worktreeId: WT, groupId: GROUP_ID })] - }, - activeWorktreeId: WT - }) + seedOmpTab(store) - for (const frame of [label, `${label} ready`, `⠋ ${label}`]) { - store.getState().setRuntimePaneTitle(TAB_ID, PANE_ID, frame) - expect(store.getState().runtimePaneTitlesByTabId[TAB_ID]?.[PANE_ID]).toBe(frame) + for (const frame of ['⠋ Pi', '⠋ OMP']) { + const fresh = createTestStore() + seedOmpTab(fresh) + fresh.getState().setRuntimePaneTitle(TAB_ID, PANE_ID, frame) + expect(fresh.getState().runtimePaneTitlesByTabId[TAB_ID]?.[PANE_ID]).toBe(frame) + fresh.getState().updateTabTitle(TAB_ID, frame) + expect(fresh.getState().tabsByWorktree[WT]?.[0]?.title).toBe(frame) } + void store + }) + + // Why: a multiplexer prefixes the pane's own title, so the identity sits mid-string. Anchored + // matching missed it and the flap survived under tmux/Zellij (#8032). + it('suppresses the flap under a multiplexer prefix', () => { + const store = createTestStore() + seedOmpTab(store) + + let commits = 0 + const unsubscribe = store.subscribe(() => { + commits += 1 + }) + for (let index = 0; index < 20; index += 1) { + const spinner = SPINNER_FRAMES[index % SPINNER_FRAMES.length] + store + .getState() + .setRuntimePaneTitle(TAB_ID, PANE_ID, `zsh | ${spinner} ${index % 2 === 0 ? 'OMP' : 'Pi'}`) + } + unsubscribe() + + expect(commits).toBeLessThanOrEqual(1) + }) + + // Why: collapsing identity must not collapse STATE — a real working->idle transition still commits. + it('still commits a genuine status change', () => { + const store = createTestStore() + seedOmpTab(store) + + store.getState().setRuntimePaneTitle(TAB_ID, PANE_ID, '⠋ OMP') + let commits = 0 + const unsubscribe = store.subscribe(() => { + commits += 1 + }) + store.getState().setRuntimePaneTitle(TAB_ID, PANE_ID, 'OMP ready') + unsubscribe() + + expect(commits).toBe(1) + expect(store.getState().runtimePaneTitlesByTabId[TAB_ID]?.[PANE_ID]).toBe('OMP ready') + }) + + // Why: a different agent taking the pane is a real identity change, not group decoration. + it('still commits a cross-group identity change', () => { + const store = createTestStore() + seedOmpTab(store) + + store.getState().setRuntimePaneTitle(TAB_ID, PANE_ID, '⠋ OMP') + let commits = 0 + const unsubscribe = store.subscribe(() => { + commits += 1 + }) + store.getState().setRuntimePaneTitle(TAB_ID, PANE_ID, '⠙ Codex') + unsubscribe() + + expect(commits).toBe(1) + expect(store.getState().runtimePaneTitlesByTabId[TAB_ID]?.[PANE_ID]).toBe('⠙ Codex') }) // Why: owner-pinning must stay scoped to bare identity frames. A semantic session title carries diff --git a/src/renderer/src/store/slices/terminal-tab-title-batch.ts b/src/renderer/src/store/slices/terminal-tab-title-batch.ts index bc49c3affe6..d0456541af3 100644 --- a/src/renderer/src/store/slices/terminal-tab-title-batch.ts +++ b/src/renderer/src/store/slices/terminal-tab-title-batch.ts @@ -1,6 +1,5 @@ import { deriveGeneratedTabTitle } from '../../../../shared/agent-tab-title' import { isDecorativeAgentTitleFrameChange } from '../../../../shared/agent-decorative-title-signature' -import { relabelCompatibleAgentIdentityFrameForOwner } from '../../../../shared/agent-title-owner' import { parseLegacyNumericPaneKey, parsePaneKey } from '../../../../shared/stable-pane-id' import type { Tab } from '../../../../shared/tab-types' import type { TerminalTab } from '../../../../shared/terminal-tab-types' @@ -180,10 +179,7 @@ export function applyTerminalTabTitleUpdates( if (!currentTab || !tabIndexes) { continue } - // Why: a wrapped harness emits the inner agent's identity (OMP wraps Pi), so pin those frames - // to the launch owner — otherwise the alternating label defeats the decorative check below. - const ownedTitle = relabelCompatibleAgentIdentityFrameForOwner(title, currentTab.launchAgent) - const nextTitle = ownedTitle.trim() || getFallbackTabTitle(currentTab) + const nextTitle = title.trim() || getFallbackTabTitle(currentTab) if (isDecorativeAgentTitleFrameChange(currentTab.title, nextTitle)) { updateStageUnifiedLabel(stage, tabId, 'label', currentTab.title) continue diff --git a/src/renderer/src/store/terminals/terminal-tab-presentation.ts b/src/renderer/src/store/terminals/terminal-tab-presentation.ts index 7ce896dcf7b..7e1d2f64442 100644 --- a/src/renderer/src/store/terminals/terminal-tab-presentation.ts +++ b/src/renderer/src/store/terminals/terminal-tab-presentation.ts @@ -1,6 +1,4 @@ import { isDecorativeAgentTitleFrameChange } from '../../../../shared/agent-decorative-title-signature' -import { relabelCompatibleAgentIdentityFrameForOwner } from '../../../../shared/agent-title-owner' -import { getPiCompatibleSyntheticAgentLabel } from '../../../../shared/pi-compatible-synthetic-title' import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' import { classifyTitleActivity } from '@/lib/pane-agent-evidence' import { @@ -12,21 +10,8 @@ import { getTerminalTabOwnerWorktreeId } from '../slices/terminal-tab-owner-index' import { getTabIdFromPaneKey } from './terminal-pty-identities' -import type { TuiAgent } from '../../../../shared/tui-agent' -import type { AppState } from '../types' import type { TerminalSlice, TerminalStoreGet, TerminalStoreSet } from './terminal-state' -function getTabLaunchAgent( - state: Pick, - tabId: string -): TuiAgent | undefined { - const ownerWorktreeId = getTerminalTabOwnerWorktreeId(state.tabsByWorktree, tabId) - if (!ownerWorktreeId) { - return undefined - } - return state.tabsByWorktree[ownerWorktreeId]?.find((tab) => tab.id === tabId)?.launchAgent -} - export function createTerminalTabPresentationActions( set: TerminalStoreSet, get: TerminalStoreGet @@ -157,13 +142,8 @@ export function createTerminalTabPresentationActions( return { tabsByWorktree: { ...s.tabsByWorktree, [ownerWorktreeId]: nextTabs } } }) }, - setRuntimePaneTitle: (tabId, paneId, rawTitle) => { + setRuntimePaneTitle: (tabId, paneId, title) => { set((s) => { - // Why: same-group frames (OMP wraps Pi) must read as the launch owner or the alternating - // label defeats the decorative check below; the owner lookup stays off the common path. - const title = getPiCompatibleSyntheticAgentLabel(rawTitle) - ? relabelCompatibleAgentIdentityFrameForOwner(rawTitle, getTabLaunchAgent(s, tabId)) - : rawTitle const currentByPane = s.runtimePaneTitlesByTabId[tabId] ?? {} const prevTitle = currentByPane[paneId] if (prevTitle === title) { diff --git a/src/shared/agent-decorative-title-signature.ts b/src/shared/agent-decorative-title-signature.ts index 317a744d098..7a2980e3a82 100644 --- a/src/shared/agent-decorative-title-signature.ts +++ b/src/shared/agent-decorative-title-signature.ts @@ -1,4 +1,5 @@ import { detectAgentStatusFromTitle, MAX_OSC_TITLE_CHARS } from './agent-detection' +import { collapseCompatibleAgentTitleIdentity } from './agent-title-owner' export const DECORATIVE_AGENT_TITLE_SIGNATURE_SOURCE_SCAN_LIMIT = MAX_OSC_TITLE_CHARS @@ -13,8 +14,11 @@ export function getDecorativeAgentTitleSignature(title: string): string | null { return null } // Why: agent spinners can emit OSC title frames many times per second; the - // spinner glyph is live decoration, not meaningful tab or sort state. - return `${status}:${normalizeDecorativeAgentTitleText(title)}` + // spinner glyph is live decoration, not meaningful tab or sort state. So is + // which member of an identity group a frame names — a wrapped harness (OMP + // wraps Pi) alternates them at spinner rate, and a literal signature would + // classify every alternation as real and commit it. + return `${status}:${normalizeDecorativeAgentTitleText(collapseCompatibleAgentTitleIdentity(title))}` } export function isDecorativeAgentTitleFrameChange(prevTitle: string, nextTitle: string): boolean { diff --git a/src/shared/agent-title-owner.ts b/src/shared/agent-title-owner.ts index edf1f945527..d2da63fc47f 100644 --- a/src/shared/agent-title-owner.ts +++ b/src/shared/agent-title-owner.ts @@ -5,10 +5,7 @@ import { SYNTHETIC_AGENT_TITLE_PROFILES, type SyntheticAgentTitleProfile } from './synthetic-agent-title' -import { - getPiCompatibleSyntheticAgentLabel, - isLegacyPiCompatibleTitle -} from './pi-compatible-synthetic-title' +import { isLegacyPiCompatibleTitle } from './pi-compatible-synthetic-title' import { getWrapperTitleSegments } from './terminal-title-wrapper-segments' type TitleProfileMatch = { @@ -69,6 +66,23 @@ export function hasCompatibleAgentTitleIdentity(title: string): boolean { return Boolean(getProfileForTitle(title)?.profile.titleIdentityGroup) } +/** + * Replaces the agent-identity segment of a title with its identity-group name, leaving any + * wrapper prefix intact ("zsh | ⠋ Pi" and "zsh | ⠋ OMP" both become "zsh | pi-compatible"). + * + * Why: members of a group are the same agent to a pane — OMP wraps Pi and both harnesses emit + * frames — so which one a frame names is decoration, not state. Callers comparing consecutive + * frames need them to compare equal; callers rendering the title must not use this. + */ +export function collapseCompatibleAgentTitleIdentity(title: string): string { + const source = getProfileForTitle(title) + const group = source?.profile.titleIdentityGroup + if (!group) { + return title + } + return title.replace(source.sourceTitle, group) +} + /** * Detects the agent status (working, permission, idle) from a terminal title, * accounting for legacy Pi titles. @@ -169,31 +183,6 @@ export function normalizeCompatibleAgentTitleForOwner( return ownerProfile.workingLabel } -/** - * Relabels only a bare identity frame ("⠋ Pi", "Pi ready") to the owner's label. - * - * Why: a wrapped harness publishes the inner agent's identity (OMP wraps Pi), so those frames - * alternate against the owner's own and defeat decorative-frame suppression at the store. A - * semantic session title ("π - - ") is left alone — it carries text no owner - * profile can reproduce, so rewriting it to a generic label would lose real information. - */ -export function relabelCompatibleAgentIdentityFrameForOwner( - title: string, - ownerAgentType: AgentType | null | undefined -): string { - const frameLabel = getPiCompatibleSyntheticAgentLabel(title) - if (!frameLabel) { - return title - } - // Why: the frame already names the owner, so its own status wording is authoritative — rewriting - // it would restate bare "Pi" as "Pi ready" and change a Pi-owned tab that never flapped. - const ownerProfile = getSyntheticAgentTitleProfile(ownerAgentType) - if (ownerProfile?.workingLabel.toLowerCase() === frameLabel.toLowerCase()) { - return title - } - return normalizeCompatibleAgentTitleForOwner(title, ownerAgentType) -} - /** * Why: mirrored remote status entries must keep the owner and title in sync * or later snapshots repaint the same tab under the wrapper agent.