diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index ffd4d453c71..5924cfa12c2 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -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() + private statusDropListeners = new Set() private statusChangeListeners = new Set() private providerSessionChangeListeners = new Set() // 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. diff --git a/src/main/index.ts b/src/main/index.ts index 94931d16803..0780aefd9b9 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -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 diff --git a/src/main/synthetic-title-spinner.test.ts b/src/main/synthetic-title-spinner.test.ts index f79afa1b2cb..633e3e069d0 100644 --- a/src/main/synthetic-title-spinner.test.ts +++ b/src/main/synthetic-title-spinner.test.ts @@ -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() + }) +}) diff --git a/src/main/synthetic-title-spinner.ts b/src/main/synthetic-title-spinner.ts index df8379a29d3..a939a73fe01 100644 --- a/src/main/synthetic-title-spinner.ts +++ b/src/main/synthetic-title-spinner.ts @@ -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 = { frame: number profile: TProfile diff --git a/src/renderer/src/components/terminal-pane/terminal-title-tracker-parity.test.ts b/src/renderer/src/components/terminal-pane/terminal-title-tracker-parity.test.ts index 6dd817ca6bf..09126e91c6b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-title-tracker-parity.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-title-tracker-parity.test.ts @@ -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}`) diff --git a/src/shared/agent-detection.test.ts b/src/shared/agent-detection.test.ts index 32fc1bd2649..e09932672db 100644 --- a/src/shared/agent-detection.test.ts +++ b/src/shared/agent-detection.test.ts @@ -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') diff --git a/src/shared/agent-title-status.ts b/src/shared/agent-title-status.ts index 82cf13bf408..423af759281 100644 --- a/src/shared/agent-title-status.ts +++ b/src/shared/agent-title-status.ts @@ -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' } diff --git a/src/shared/pi-state-title-marker.test.ts b/src/shared/pi-state-title-marker.test.ts new file mode 100644 index 00000000000..d060a67d603 --- /dev/null +++ b/src/shared/pi-state-title-marker.test.ts @@ -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) + } + }) +}) diff --git a/src/shared/pi-state-title-marker.ts b/src/shared/pi-state-title-marker.ts new file mode 100644 index 00000000000..7f6ece71bd6 --- /dev/null +++ b/src/shared/pi-state-title-marker.ts @@ -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 + +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)}` +}