mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
fix(terminal): collapse identity group in the title churn signature
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>
This commit is contained in:
@@ -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;<frame> <profile.workingLabel>\x07`
|
||||
* (:2170, :2205). For an OMP-owned pane the profile is `omp`, so every tick
|
||||
* asserts "<spinner> OMP".
|
||||
* shared 80ms interval (src/main/index.ts SPINNER_INTERVAL_MS) that injects
|
||||
* `\x1b]0;<frame> <profile.workingLabel>\x07`. For an OMP-owned pane the
|
||||
* profile is `omp`, so every tick asserts "<spinner> 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<AppState, 'tabsByWorktree'>,
|
||||
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) {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 ("π - <session> - <cwd>") 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.
|
||||
|
||||
Reference in New Issue
Block a user