mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
fix(omp): read Pi/OMP static state-title markers and retire stale spinners (#14602)
* fix(omp): read Pi/OMP static state-title markers and retire stale spinners OMP 17.2.12 replaced its animated braille title frames with static markers on WSL/ConPTY (`π : working`, `π > idle`, `π ! needs input`). Orca read all three as idle, so a working OMP pane lost its status, and a synthetic title spinner started by an earlier hook kept rotating after its status row was gone. Classify the markers from one shared table so a later upstream punctuation change is a row, not a reparse, and stop the spinner when the hook row it stands in for is cleared or dismissed. Fixes #13890 * test(omp): preserve static state titles during normalization --------- Co-authored-by: Merge Sim <sim@local>
This commit is contained in:
co-authored by
Merge Sim
parent
60a7ac3804
commit
e54cfc1901
@@ -201,6 +201,7 @@ export type AgentHookAuthorityAttestation = Readonly<{
|
||||
type StatusChangeListener = (statuses: AgentHookStatusChangeEntry[]) => void
|
||||
type ProviderSessionChangeListener = (providerSessions: AgentHookProviderSessionIdentity[]) => void
|
||||
type PaneStatusClearListener = (clear: AgentStatusClearIpcPayload) => void
|
||||
type StatusDropListener = (paneKey: string) => void
|
||||
type PaneKeyAliasPersistenceListener = (entries: LegacyPaneKeyAliasEntry[]) => void
|
||||
type PaneKeyAliasEntry = {
|
||||
stablePaneKey: string
|
||||
@@ -705,6 +706,7 @@ export class AgentHookServer {
|
||||
private onClaudeStatusLine: ((event: ClaudeStatusLineRateLimits) => void) | null = null
|
||||
private onPaneStatusCleared: PaneStatusClearListener | null = null
|
||||
private paneStatusClearListeners = new Set<PaneStatusClearListener>()
|
||||
private statusDropListeners = new Set<StatusDropListener>()
|
||||
private statusChangeListeners = new Set<StatusChangeListener>()
|
||||
private providerSessionChangeListeners = new Set<ProviderSessionChangeListener>()
|
||||
// Why: setListener is a single slot owned by the main-window fanout; the
|
||||
@@ -864,6 +866,27 @@ export class AgentHookServer {
|
||||
}
|
||||
}
|
||||
|
||||
/** Multi-subscriber tap on definitive live-row deletions. `dropStatusEntry` is a user
|
||||
* dismissal, so it never routes through the pane-status-clear fan-out — pane-owned
|
||||
* cleanup (synthetic spinners) still has to retire with the row it was driving. */
|
||||
subscribeStatusDrop(listener: StatusDropListener): () => void {
|
||||
this.statusDropListeners.add(listener)
|
||||
return () => {
|
||||
this.statusDropListeners.delete(listener)
|
||||
}
|
||||
}
|
||||
|
||||
private emitStatusDropped(paneKey: string): void {
|
||||
for (const listener of this.statusDropListeners) {
|
||||
// Why: matches every other fan-out here — one throwing subscriber must not strand the rest.
|
||||
try {
|
||||
listener(paneKey)
|
||||
} catch (err) {
|
||||
console.error('[agent-hooks] status-drop listener threw', err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private emitPaneStatusCleared(clear: AgentStatusClearIpcPayload): void {
|
||||
this.onPaneStatusCleared?.(clear)
|
||||
for (const listener of this.paneStatusClearListeners) {
|
||||
@@ -2767,6 +2790,7 @@ export class AgentHookServer {
|
||||
}
|
||||
this.scheduleStatusPersist()
|
||||
this.notifyStatusChangeListeners()
|
||||
this.emitStatusDropped(deleted.paneKey)
|
||||
}
|
||||
|
||||
/** Retire panes whose owning process is certifiably dead.
|
||||
|
||||
@@ -386,6 +386,7 @@ import { startPreGoneProcessMetricsSampling } from './crash-reporting/process-go
|
||||
import { resolveExpectedTeardownScope } from './crash-reporting/expected-teardown-state'
|
||||
import {
|
||||
advanceSyntheticTitleSpinnerEntries,
|
||||
getSyntheticTitleSpinnerPaneKeyToStop,
|
||||
type SyntheticTitleSpinnerEntry
|
||||
} from './synthetic-title-spinner'
|
||||
import { shouldSendSyntheticTitleFrame } from './synthetic-title-visibility'
|
||||
@@ -2208,6 +2209,18 @@ registerPaneKeyTeardownListener((paneKey) => {
|
||||
stopSyntheticTitleSpinner(paneKey)
|
||||
})
|
||||
|
||||
// Why: the spinner is a stand-in for a live hook status, so it must retire with the row it
|
||||
// stands in for — otherwise a pane whose status was cleared or dismissed keeps rotating a
|
||||
// working title long after the agent finished (#13890). Both paths are covered: the
|
||||
// pane-scoped clear fan-out, and user dismissal, which never routes through it.
|
||||
agentHookServer.subscribePaneStatusClear((clear) => {
|
||||
const paneKey = getSyntheticTitleSpinnerPaneKeyToStop(clear)
|
||||
if (paneKey) {
|
||||
stopSyntheticTitleSpinner(paneKey)
|
||||
}
|
||||
})
|
||||
agentHookServer.subscribeStatusDrop(stopSyntheticTitleSpinner)
|
||||
|
||||
function sendSyntheticTitle(ptyId: string, data: string, options: { force?: boolean } = {}): void {
|
||||
if (!mainWindow || mainWindow.isDestroyed()) {
|
||||
return
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
advanceSyntheticTitleSpinnerEntries,
|
||||
getSyntheticTitleSpinnerPaneKeyToStop,
|
||||
type SyntheticTitleSpinnerEntry
|
||||
} from './synthetic-title-spinner'
|
||||
|
||||
@@ -44,3 +45,21 @@ describe('advanceSyntheticTitleSpinnerEntries', () => {
|
||||
expect(entries.has('stale-pane')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getSyntheticTitleSpinnerPaneKeyToStop', () => {
|
||||
// Why: without this the spinner outlived the hook row it stood in for, so a finished
|
||||
// OMP pane kept rotating a working title (#13890).
|
||||
it('retires the spinner of a pane-scoped clear', () => {
|
||||
expect(getSyntheticTitleSpinnerPaneKeyToStop({ paneKey: 'pane-a' })).toBe('pane-a')
|
||||
})
|
||||
|
||||
it('keeps pane spinners running for a connection-scoped transient clear', () => {
|
||||
expect(
|
||||
getSyntheticTitleSpinnerPaneKeyToStop({
|
||||
transient: true,
|
||||
connectionId: 'ssh-a',
|
||||
clearedAt: 42
|
||||
})
|
||||
).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,3 +1,16 @@
|
||||
import type { AgentStatusClearIpcPayload } from '../shared/agent-status-types'
|
||||
|
||||
/**
|
||||
* Pane whose synthetic spinner a hook-status clear retires, or null to leave every spinner
|
||||
* running. Connection-scoped transient clears carry no pane key on purpose: losing an SSH
|
||||
* transport is not evidence that the remote PTY stopped working, so they must not idle it.
|
||||
*/
|
||||
export function getSyntheticTitleSpinnerPaneKeyToStop(
|
||||
clear: AgentStatusClearIpcPayload
|
||||
): string | null {
|
||||
return 'paneKey' in clear ? clear.paneKey : null
|
||||
}
|
||||
|
||||
export type SyntheticTitleSpinnerEntry<TProfile> = {
|
||||
frame: number
|
||||
profile: TProfile
|
||||
|
||||
@@ -119,6 +119,32 @@ describe('main title tracker parity with the renderer transport processor', () =
|
||||
expect(kinds.indexOf('became-working')).toBeLessThan(kinds.indexOf('became-idle'))
|
||||
})
|
||||
|
||||
// Why: OMP 17.2.12+ cannot animate under WSL/ConPTY, so it emits static state markers
|
||||
// instead of braille frames (#13890). Both paths must see the same working→idle turn.
|
||||
it('derives identical facts from static OMP WSL state titles', () => {
|
||||
const chunk = `${ESC}]0;zsh | π : cwd${BEL}response text\r\n` + `${ESC}]0;zsh | π > cwd${BEL}`
|
||||
feedBoth(paths, chunk)
|
||||
|
||||
expect(paths.main.events).toEqual(paths.renderer.events)
|
||||
const kinds = paths.main.events.map((event) => event.kind)
|
||||
expect(kinds).toContain('became-working')
|
||||
expect(kinds.indexOf('became-working')).toBeLessThan(kinds.indexOf('became-idle'))
|
||||
})
|
||||
|
||||
// Why: an OMP pane that stops emitting titles mid-turn must still leave working, or the
|
||||
// stale native marker keeps the pane — and its synthetic spinner — pinned to working.
|
||||
it('clears a stale static OMP working title in both paths', () => {
|
||||
feedBoth(paths, `${ESC}]0;zsh | π : cwd${BEL}`)
|
||||
feedBoth(paths, 'title-free output')
|
||||
vi.advanceTimersByTime(3_000)
|
||||
|
||||
expect(paths.main.events).toEqual(paths.renderer.events)
|
||||
expect(paths.main.events).toContainEqual({
|
||||
kind: 'became-idle',
|
||||
title: 'zsh | π > cwd'
|
||||
})
|
||||
})
|
||||
|
||||
it('derives identical facts from BEL- and ST-terminated titles', () => {
|
||||
feedBoth(paths, `${ESC}]2;Codex working${ST}body bytes`)
|
||||
feedBoth(paths, `${ESC}]0;Codex done${BEL}`)
|
||||
|
||||
@@ -167,6 +167,20 @@ describe('Pi-compatible title detection', () => {
|
||||
expect(detectAgentStatusFromTitle(title)).toBe(expectedStatus)
|
||||
})
|
||||
|
||||
// Why: OMP 17.2.12+ writes static state markers on WSL/ConPTY (#13890). The label after
|
||||
// the marker is cwd/session text, so glyphs another agent uses for status must not win.
|
||||
it.each([
|
||||
['π : my-project', 'working', 'π : my-project'],
|
||||
['π > my-project', 'idle', 'π > my-project'],
|
||||
['π ! my-project', 'permission', 'π ! my-project'],
|
||||
['π > gemini ✦ ⏲ ◇ ✋', 'idle', 'π > gemini ✦ ⏲ ◇ ✋'],
|
||||
['zsh | π : my-project', 'working', 'zsh | π : my-project'],
|
||||
['zsh | π > gemini ✦ ⏲ ◇ ✋', 'idle', 'zsh | π > gemini ✦ ⏲ ◇ ✋']
|
||||
] as const)('classifies native OMP title %j as %s', (title, expectedStatus, expectedDisplay) => {
|
||||
expect(detectAgentStatusFromTitle(title)).toBe(expectedStatus)
|
||||
expect(normalizeTerminalTitle(title)).toBe(expectedDisplay)
|
||||
})
|
||||
|
||||
it('re-detects status after display-title normalization for Pi idle frames', () => {
|
||||
// Normalization now preserves the session name and cwd (#16093).
|
||||
expect(normalizeTerminalTitle('π - my-project')).toBe('π - my-project')
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
getPiCompatibleTitleSeparatorStatus,
|
||||
getPiCompatibleSyntheticAgentStatus
|
||||
} from './pi-compatible-synthetic-title'
|
||||
import { clearPiStateWorkingMarker, getPiStateTitleStatus } from './pi-state-title-marker'
|
||||
import { getWrapperTitleSegments } from './terminal-title-wrapper-segments'
|
||||
import { isGrokRotatingWorkingTitle } from './terminal-title-agent-type'
|
||||
|
||||
@@ -36,6 +37,13 @@ import { isGrokRotatingWorkingTitle } from './terminal-title-agent-type'
|
||||
* Strip working-status indicators so stale exit titles stop reporting working.
|
||||
*/
|
||||
export function clearWorkingIndicators(title: string): string {
|
||||
// Why: Pi/OMP's static working marker survives every strip below, so a stale native
|
||||
// title would keep re-arming the 3s clear timer without ever leaving working (#13890).
|
||||
const clearedPiStateMarker = clearPiStateWorkingMarker(title)
|
||||
if (clearedPiStateMarker) {
|
||||
return clearedPiStateMarker
|
||||
}
|
||||
|
||||
let cleaned = title
|
||||
|
||||
cleaned = cleaned.replace(GEMINI_WORKING, '')
|
||||
@@ -123,7 +131,9 @@ export function normalizeTerminalTitle(title: string): string {
|
||||
return title
|
||||
}
|
||||
|
||||
if (isGeminiTerminalTitle(title)) {
|
||||
// Why: a Pi/OMP label is cwd/session text that may contain Gemini's glyphs; its own
|
||||
// state marker is explicit, so it outranks glyph sniffing here as it does in detection.
|
||||
if (!getPiStateTitleStatus(title) && isGeminiTerminalTitle(title)) {
|
||||
const status = detectAgentStatusFromTitle(title)
|
||||
if (status === 'permission') {
|
||||
return `${GEMINI_PERMISSION} Gemini CLI`
|
||||
@@ -180,6 +190,13 @@ export function detectAgentStatusFromTitle(title: string): AgentStatus | null {
|
||||
return containsAgentSpinnerGlyph(title) ? 'working' : 'idle'
|
||||
}
|
||||
|
||||
// Why: Pi/OMP's marker is an explicit state protocol, so it wins over the glyph and
|
||||
// keyword gates below — its label is free-form cwd/session text that can carry either.
|
||||
const piStateStatus = getPiStateTitleStatus(title)
|
||||
if (piStateStatus) {
|
||||
return piStateStatus
|
||||
}
|
||||
|
||||
if (title.includes(GEMINI_PERMISSION)) {
|
||||
return 'permission'
|
||||
}
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { clearWorkingIndicators, detectAgentStatusFromTitle } from './agent-detection'
|
||||
import {
|
||||
clearPiStateWorkingMarker,
|
||||
getPiStateTitleStatus,
|
||||
PI_STATE_MARKERS
|
||||
} from './pi-state-title-marker'
|
||||
|
||||
// Why: OMP 17.2.12 swapped its animated braille frames for static markers on WSL/ConPTY
|
||||
// (#13890, upstream #8014). Every case below was idle before the marker table existed.
|
||||
describe('Pi/OMP native state-title markers', () => {
|
||||
it.each([
|
||||
// bare 17.2.12+ titles
|
||||
['π : my-project', 'working'],
|
||||
['π > my-project', 'idle'],
|
||||
['π ! my-project', 'permission'],
|
||||
// wrapper-prefixed titles — multiplexers own the head of the string
|
||||
['zsh | π : my-project', 'working'],
|
||||
['zsh | tmux | π > my-project', 'idle'],
|
||||
['zsh | π ! my-project', 'permission'],
|
||||
['tmux: π : my-project', 'working'],
|
||||
// the label is opaque: it legally carries the wrapper separator and more markers
|
||||
['π > release | π : note | π ! note', 'idle'],
|
||||
['zsh | π : release | π > note', 'working'],
|
||||
// ...and glyphs another agent uses for status
|
||||
['π > session ✦ ⏲ ◇ ✋', 'idle'],
|
||||
['π : session ✦ ⏲ ◇ ✋', 'working'],
|
||||
// no label at all
|
||||
['π :', 'working'],
|
||||
['π >', 'idle']
|
||||
] as const)('classifies %j as %s', (title, expected) => {
|
||||
expect(getPiStateTitleStatus(title)).toBe(expected)
|
||||
expect(detectAgentStatusFromTitle(title)).toBe(expected)
|
||||
})
|
||||
|
||||
it.each([
|
||||
// Why: the legacy no-space form is OMP's disabled title, not a working marker. It has
|
||||
// always classified idle and must keep doing so on hosts still running 17.2.11.
|
||||
['π: my-project', 'idle'],
|
||||
['π - my-project', 'idle'],
|
||||
['⠋ π - my-project', 'working']
|
||||
] as const)('leaves the legacy title %j on its historical path (%s)', (title, expected) => {
|
||||
expect(getPiStateTitleStatus(title)).toBeNull()
|
||||
expect(detectAgentStatusFromTitle(title)).toBe(expected)
|
||||
})
|
||||
|
||||
it.each(['πx : glued', 'not-a-marker', ''])(
|
||||
'claims no marker in %j so other detectors stay authoritative',
|
||||
(title) => {
|
||||
expect(getPiStateTitleStatus(title)).toBeNull()
|
||||
}
|
||||
)
|
||||
|
||||
// Why: an unrecognized marker means a protocol Orca has not been taught yet. Claiming a
|
||||
// status from it would repeat this bug in the other direction, so the parser abstains
|
||||
// and the pre-existing Pi gates decide.
|
||||
it('abstains on a marker the table does not define', () => {
|
||||
expect(PI_STATE_MARKERS).not.toContain('~')
|
||||
expect(getPiStateTitleStatus('π ~ my-project')).toBeNull()
|
||||
})
|
||||
|
||||
describe('stale working-title clear', () => {
|
||||
it.each([
|
||||
['π : my-project', 'π > my-project'],
|
||||
['zsh | π : release | π : note', 'zsh | π > release | π : note']
|
||||
] as const)('rewrites %j to an idle marker', (title, expected) => {
|
||||
expect(clearPiStateWorkingMarker(title)).toBe(expected)
|
||||
// The 3s stale-title fallback re-detects what it wrote; it must not still be working.
|
||||
expect(clearWorkingIndicators(title)).toBe(expected)
|
||||
expect(detectAgentStatusFromTitle(clearWorkingIndicators(title))).toBe('idle')
|
||||
})
|
||||
|
||||
it.each(['π > my-project', 'π ! my-project', '⠋ Pi'])(
|
||||
'leaves %j to the other strip passes',
|
||||
(title) => {
|
||||
expect(clearPiStateWorkingMarker(title)).toBeNull()
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
// Why: the table is the single source of truth. A new marker added there must reach
|
||||
// detection without editing this file, or the two drift the way they did in #13890.
|
||||
it('routes every table marker through title detection', () => {
|
||||
for (const marker of PI_STATE_MARKERS) {
|
||||
const status = getPiStateTitleStatus(`π ${marker} my-project`)
|
||||
expect(status).not.toBeNull()
|
||||
expect(detectAgentStatusFromTitle(`π ${marker} my-project`)).toBe(status)
|
||||
expect(detectAgentStatusFromTitle(`zsh | π ${marker} my-project`)).toBe(status)
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,75 @@
|
||||
import type { AgentStatus } from './agent-title-core'
|
||||
|
||||
/**
|
||||
* Markers Pi/OMP write between the `π` prefix and their label to encode turn state
|
||||
* (`π : cwd` working, `π > cwd` idle, `π ! cwd` needs input).
|
||||
*
|
||||
* Kept as a table because upstream re-punctuates this channel between releases: OMP
|
||||
* 17.2.12 replaced its animated braille frames with these static markers on WSL/ConPTY,
|
||||
* where the console host cannot repaint fast enough to animate (#13890, #8014). Every
|
||||
* consumer — status detection, the display-title normalizer, and the stale-title clear —
|
||||
* reads this one table, so teaching Orca a later protocol is a row, not a reparse.
|
||||
*/
|
||||
const PI_STATE_MARKER_STATUS = {
|
||||
':': 'working',
|
||||
'!': 'permission',
|
||||
'>': 'idle'
|
||||
} as const satisfies Record<string, AgentStatus>
|
||||
|
||||
export type PiStateMarker = keyof typeof PI_STATE_MARKER_STATUS
|
||||
|
||||
export const PI_STATE_MARKERS = Object.keys(PI_STATE_MARKER_STATUS) as PiStateMarker[]
|
||||
|
||||
/** Marker a stale working title is rewritten to; see {@link clearPiStateWorkingMarker}. */
|
||||
const PI_IDLE_MARKER = '>' satisfies PiStateMarker
|
||||
|
||||
function escapeForCharacterClass(marker: string): string {
|
||||
return marker.replace(/[\\\]^-]/g, '\\$&')
|
||||
}
|
||||
|
||||
// Why: `π` must sit at a token boundary so wrapper prefixes of any shape (`zsh | π : cwd`,
|
||||
// `tmux: π : cwd`) still expose the marker, and whitespace must separate the marker so the
|
||||
// legacy no-space `π: cwd` disabled title keeps its historical idle classification.
|
||||
const PI_STATE_TITLE_RE = new RegExp(
|
||||
`(?:^|[\\s|])π[ \\t]+([${PI_STATE_MARKERS.map(escapeForCharacterClass).join('')}])(?=\\s|$)`,
|
||||
'u'
|
||||
)
|
||||
|
||||
type PiStateTitleMatch = {
|
||||
marker: PiStateMarker
|
||||
markerIndex: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Leftmost marker wins: everything after it is Pi/OMP's own label, which legally contains
|
||||
* the wrapper separator and marker-shaped punctuation of its own (`π > release | π : note`).
|
||||
*/
|
||||
function matchPiStateTitle(title: string): PiStateTitleMatch | null {
|
||||
const match = PI_STATE_TITLE_RE.exec(title)
|
||||
if (!match) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
marker: match[1] as PiStateMarker,
|
||||
markerIndex: match.index + match[0].length - 1
|
||||
}
|
||||
}
|
||||
|
||||
/** Status a Pi/OMP native state title asserts, or null when the title carries no marker. */
|
||||
export function getPiStateTitleStatus(title: string): AgentStatus | null {
|
||||
const match = matchPiStateTitle(title)
|
||||
return match ? PI_STATE_MARKER_STATUS[match.marker] : null
|
||||
}
|
||||
|
||||
/**
|
||||
* Rewrite a working marker to the idle marker so a title left behind by an agent that
|
||||
* stopped emitting stops reporting working. Returns null when there is nothing to clear —
|
||||
* the caller's other strip passes still apply.
|
||||
*/
|
||||
export function clearPiStateWorkingMarker(title: string): string | null {
|
||||
const match = matchPiStateTitle(title)
|
||||
if (!match || PI_STATE_MARKER_STATUS[match.marker] !== 'working') {
|
||||
return null
|
||||
}
|
||||
return `${title.slice(0, match.markerIndex)}${PI_IDLE_MARKER}${title.slice(match.markerIndex + 1)}`
|
||||
}
|
||||
Reference in New Issue
Block a user