diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 2fd94df75..8c41f2569 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -8468,6 +8468,81 @@ describe('OrcaRuntimeService', () => { expect(events).toHaveLength(1) }) + it('omits terminalSideEffects from non-consuming listeners while other events still flow', () => { + const runtime = new OrcaRuntimeService(store) + const desktopEvents: RuntimeClientEvent[] = [] + const mobileEvents: RuntimeClientEvent[] = [] + runtime.syncWindowGraph(HEADLESS_RUNTIME_WINDOW_ID, { tabs: [], leaves: [] }) + runtime.onClientEvent((event) => desktopEvents.push(event)) + runtime.onClientEvent((event) => mobileEvents.push(event), { + consumesTerminalSideEffects: false + }) + + runtime.onPtyData('pty-remote', '\x1b]0;Codex working\x07', 100) + runtime.notifyBranchRenamed(TEST_REPO_ID) + + expect(desktopEvents.map((event) => event.type)).toEqual([ + 'terminalSideEffects', + 'worktreesChanged' + ]) + expect(mobileEvents.map((event) => event.type)).toEqual(['worktreesChanged']) + }) + + it('keeps a phone-only host producing title state without emitting batches to it', async () => { + vi.useFakeTimers() + try { + const ptyId = `${TEST_REPO_ID}::/tmp/worktree-a@@pty-a` + const runtime = new OrcaRuntimeService(store) + const mobileEvents: RuntimeClientEvent[] = [] + const trackerEntries = ( + runtime as unknown as { + ptyTitleTrackersByPtyId: Map + } + ).ptyTitleTrackersByPtyId + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses: async () => [{ id: ptyId, cwd: '/tmp/worktree-a', title: 'shell' }] + }) + runtime.syncWindowGraph(HEADLESS_RUNTIME_WINDOW_ID, { tabs: [], leaves: [] }) + runtime.onClientEvent((event) => mobileEvents.push(event), { + consumesTerminalSideEffects: false + }) + const unsubscribeDesktop = runtime.onClientEvent(() => {}) + + runtime.onPtyData(ptyId, '\x1b]0;Codex working\x07', 100) + runtime.onPtyData(ptyId, 'output without a title\r\n', 101) + // The phone is still subscribed: disposing trackers on this edge would cancel + // its armed stale-working-title timer and strand a 'working' spinner (#1437). + unsubscribeDesktop() + + await vi.advanceTimersByTimeAsync(3_000) + + expect(trackerEntries.has(ptyId)).toBe(true) + expect(trackerEntries.get(ptyId)?.commandCodeDetector).toBeNull() + expect((await runtime.listTerminals()).terminals[0]).toMatchObject({ title: 'Codex' }) + expect(mobileEvents.some((event) => event.type === 'terminalSideEffects')).toBe(false) + } finally { + vi.useRealTimers() + } + }) + + it('skips a listener unsubscribed mid-fan-out even with mobile exclusions active', () => { + const runtime = new OrcaRuntimeService(store) + const lateEvents: RuntimeClientEvent[] = [] + runtime.syncWindowGraph(HEADLESS_RUNTIME_WINDOW_ID, { tabs: [], leaves: [] }) + runtime.onClientEvent(() => {}, { consumesTerminalSideEffects: false }) + runtime.onClientEvent(() => { + unsubscribeLate() + }) + const unsubscribeLate = runtime.onClientEvent((event) => lateEvents.push(event)) + + runtime.onPtyData('pty-remote', '\x1b]0;Codex working\x07', 100) + + expect(lateEvents).toEqual([]) + }) + it('emits one batched event per chunk with facts in byte order and attribution', () => { const { runtime, batches } = createSideEffectRuntime() syncSinglePty(runtime) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index b505b778e..1328dc5c6 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2725,6 +2725,10 @@ export class OrcaRuntimeService { private ptyController: RuntimePtyController | null = null private notifier: RuntimeNotifier | null = null private clientEventListeners = new Set<(event: RuntimeClientEvent) => void>() + // Why: mobile subscribers discard terminalSideEffects; exclude them from batch delivery and production. + private terminalSideEffectExcludedClientEventListeners = new Set< + (event: RuntimeClientEvent) => void + >() private nativeChatLaunchDraftResolutionByTabId = new Map< string, NativeChatLaunchDraftResolutionTombstone @@ -4681,15 +4685,26 @@ export class OrcaRuntimeService { } } - onClientEvent(listener: (event: RuntimeClientEvent) => void): () => void { + onClientEvent( + listener: (event: RuntimeClientEvent) => void, + options?: { consumesTerminalSideEffects?: boolean } + ): () => void { this.clientEventListeners.add(listener) + if (options?.consumesTerminalSideEffects === false) { + this.terminalSideEffectExcludedClientEventListeners.add(listener) + } this.refreshTerminalSideEffectConsumerAvailability() return () => { this.clientEventListeners.delete(listener) + this.terminalSideEffectExcludedClientEventListeners.delete(listener) this.refreshTerminalSideEffectConsumerAvailability() } } + private countTerminalSideEffectConsumingClientEventListeners(): number { + return this.clientEventListeners.size - this.terminalSideEffectExcludedClientEventListeners.size + } + getTerminalSleepClientEventSnapshot(): RuntimeClientEvent[] { const events: RuntimeClientEvent[] = [] const sleepStates = [...this.terminalSleepStateByWorktreeId.values()].sort((a, b) => @@ -4747,9 +4762,25 @@ export class OrcaRuntimeService { } private emitClientEvent(event: RuntimeClientEvent): void { + // Why: mobile streams discard terminalSideEffects; skip excluded listeners so + // paired phones never receive the per-OSC batch frames over the relay. Filtered + // inside the delivery callback to keep live-Set iteration (a listener that + // unsubscribes mid-fan-out must not be delivered to) and stay allocation-free. + const skipExcluded = + event.type === 'terminalSideEffects' && + this.terminalSideEffectExcludedClientEventListeners.size > 0 // Why: a throwing subscriber here once escaped acquireWorktreeTerminalSpawn after it took the // per-worktree terminal mutation, leaking it and wedging that worktree's sleep until restart. - notifyRuntimeListeners(this.clientEventListeners, (listener) => listener(event), 'client-event') + notifyRuntimeListeners( + this.clientEventListeners, + (listener) => { + if (skipExcluded && this.terminalSideEffectExcludedClientEventListeners.has(listener)) { + return + } + listener(event) + }, + 'client-event' + ) } notifyNativeChatLaunchDraftResolved( @@ -9417,7 +9448,7 @@ export class OrcaRuntimeService { console.error('[runtime] terminal side-effect listener threw', { ptyId, err }) } } - if (this.clientEventListeners.size > 0) { + if (this.countTerminalSideEffectConsumingClientEventListeners() > 0) { this.emitClientEvent({ type: 'terminalSideEffects', batch }) } } @@ -9585,33 +9616,23 @@ export class OrcaRuntimeService { this.retirePtyAgentLaunchAuthority(ptyId) this.recordTerminalSideEffectFact(ptyId, { kind: 'command-finished', exitCode }) }, - // Why: headless serve still scans command completion to retire agent - // launch authority; other transient facts remain desktop-only. - ...(this.terminalSideEffectConsumerAvailable - ? { - onBell: () => { - this.recordTerminalSideEffectFact(ptyId, { kind: 'bell' }) - }, - onPrLink: (link: TerminalGitHubPRLink) => { - this.recordTerminalSideEffectFact(ptyId, { kind: 'pr-link', link }) - }, - // Why: hidden-delivery-gated views never see the bytes, so main - // surfaces DECSET 2031 subscribes as facts; the theme reply is - // still sent by the renderer (query authority stays with the view). - onMode2031Subscribe: () => { - this.recordTerminalSideEffectFact(ptyId, { kind: '2031-subscribe' }) - }, - // Why: the gated view never sees the withdrawal bytes either, so the - // subscription registry it keeps for theme flips needs this fact to - // stay truthful. - onMode2031Unsubscribe: () => { - this.recordTerminalSideEffectFact(ptyId, { kind: '2031-unsubscribe' }) - } - } - : {}) + onBell: () => { + this.recordTerminalSideEffectFact(ptyId, { kind: 'bell' }) + }, + onPrLink: (link: TerminalGitHubPRLink) => { + this.recordTerminalSideEffectFact(ptyId, { kind: 'pr-link', link }) + }, + // Why: hidden-delivery-gated views never see 2031 bytes; facts keep their theme registry truthful. + onMode2031Subscribe: () => { + this.recordTerminalSideEffectFact(ptyId, { kind: '2031-subscribe' }) + }, + onMode2031Unsubscribe: () => { + this.recordTerminalSideEffectFact(ptyId, { kind: '2031-unsubscribe' }) + } }, initialTitle !== null ? { initialTitle } : {} ) + tracker.setTransientSideEffectScanningEnabled(this.terminalSideEffectConsumerAvailable) const entry: RuntimePtyTitleTrackerEntry = { tracker, applyingChunk: false, @@ -9624,15 +9645,7 @@ export class OrcaRuntimeService { // self-arms on the Command Code banner; the spawn command (when main // saw one) mirrors the renderer detector's startupCommand fast-arm. commandCodeDetector: this.terminalSideEffectConsumerAvailable - ? createCommandCodeOutputStatusDetector({ - startupCommand: this.terminalSpawnCommandsByPtyId.get(ptyId) ?? null, - onWorking: (prompt) => { - this.recordTerminalSideEffectFact(ptyId, { kind: 'command-code-working', prompt }) - }, - onDone: (prompt) => { - this.recordTerminalSideEffectFact(ptyId, { kind: 'command-code-done', prompt }) - } - }) + ? this.createTerminalSideEffectCommandCodeDetector(ptyId) : null } this.ptyTitleTrackersByPtyId.set(ptyId, entry) @@ -9752,18 +9765,34 @@ export class OrcaRuntimeService { private refreshTerminalSideEffectConsumerAvailability(): void { const nextAvailable = - this.terminalSideEffectLocalConsumerAvailable || this.clientEventListeners.size > 0 + this.terminalSideEffectLocalConsumerAvailable || + this.countTerminalSideEffectConsumingClientEventListeners() > 0 if (nextAvailable === this.terminalSideEffectConsumerAvailable) { return } this.terminalSideEffectConsumerAvailable = nextAvailable - // Why: optional bell/command/link scanners are selected when a tracker is - // created. Rebuild at the window boundary so pure headless output stays cheap. - for (const ptyId of [...this.ptyTitleTrackersByPtyId.keys()]) { - this.disposePtyTitleTracker(ptyId) + for (const [ptyId, entry] of this.ptyTitleTrackersByPtyId) { + entry.tracker.setTransientSideEffectScanningEnabled(nextAvailable) + entry.commandCodeDetector = nextAvailable + ? this.createTerminalSideEffectCommandCodeDetector(ptyId) + : null } } + private createTerminalSideEffectCommandCodeDetector( + ptyId: string + ): NonNullable { + return createCommandCodeOutputStatusDetector({ + startupCommand: this.terminalSpawnCommandsByPtyId.get(ptyId) ?? null, + onWorking: (prompt) => { + this.recordTerminalSideEffectFact(ptyId, { kind: 'command-code-working', prompt }) + }, + onDone: (prompt) => { + this.recordTerminalSideEffectFact(ptyId, { kind: 'command-code-done', prompt }) + } + }) + } + private extractLastOsc7CwdForPty( ptyId: string, data: string diff --git a/src/main/runtime/rpc/methods/client-events.test.ts b/src/main/runtime/rpc/methods/client-events.test.ts new file mode 100644 index 000000000..bf5026c81 --- /dev/null +++ b/src/main/runtime/rpc/methods/client-events.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RuntimeClientEvent } from '../../../../shared/runtime-client-events' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { isStreamingMethod, type RpcContext, type RpcStreamingMethod } from '../core' +// Why: importing client-events directly trips its module-init cycle through ipc/ssh; the index resolves it. +import { ALL_RPC_METHODS } from './index' + +const subscribeMethod = ALL_RPC_METHODS.find( + (method) => method.name === 'runtime.clientEvents.subscribe' && isStreamingMethod(method) +) as RpcStreamingMethod + +function makeRuntime(): { + runtime: OrcaRuntimeService + onClientEvent: ReturnType + cleanups: (() => void)[] +} { + const cleanups: (() => void)[] = [] + const onClientEvent = vi.fn( + ( + _listener: (event: RuntimeClientEvent) => void, + _options?: { consumesTerminalSideEffects?: boolean } + ) => + () => {} + ) + const runtime = { + onClientEvent, + registerSubscriptionCleanup: (_id: string, cleanup: () => void) => { + cleanups.push(cleanup) + } + } as unknown as OrcaRuntimeService + return { runtime, onClientEvent, cleanups } +} + +describe('runtime.clientEvents.subscribe', () => { + it('registers mobile subscriptions as non-consumers of terminal side effects', async () => { + const { runtime, onClientEvent, cleanups } = makeRuntime() + + const done = subscribeMethod.handler( + undefined, + { runtime, connectionId: 'conn-1', clientKind: 'mobile' } as RpcContext, + () => {} + ) + + expect(onClientEvent).toHaveBeenCalledWith(expect.any(Function), { + consumesTerminalSideEffects: false + }) + cleanups.forEach((cleanup) => cleanup()) + await done + }) + + it('keeps non-mobile subscriptions consuming terminal side effects', async () => { + const { runtime, onClientEvent, cleanups } = makeRuntime() + + const done = subscribeMethod.handler( + undefined, + { runtime, connectionId: 'conn-1' } as RpcContext, + () => {} + ) + + expect(onClientEvent).toHaveBeenCalledWith(expect.any(Function), { + consumesTerminalSideEffects: true + }) + cleanups.forEach((cleanup) => cleanup()) + await done + }) +}) diff --git a/src/main/runtime/rpc/methods/client-events.ts b/src/main/runtime/rpc/methods/client-events.ts index 956594dd5..81ee32d19 100644 --- a/src/main/runtime/rpc/methods/client-events.ts +++ b/src/main/runtime/rpc/methods/client-events.ts @@ -16,11 +16,16 @@ export const CLIENT_EVENT_METHODS: readonly RpcAnyMethod[] = [ defineStreamingMethod({ name: 'runtime.clientEvents.subscribe', params: null, - handler: async (_params, { runtime, connectionId }, emit) => { + handler: async (_params, { runtime, connectionId, clientKind }, emit) => { await new Promise((resolve) => { - const unsubscribe = runtime.onClientEvent((event) => { - emit(event) - }) + // Why: mobile discards terminalSideEffects; excluding it stops the + // per-OSC batch frames from crossing the relay. + const unsubscribe = runtime.onClientEvent( + (event) => { + emit(event) + }, + { consumesTerminalSideEffects: clientKind !== 'mobile' } + ) const seq = ++clientEventSubscriptionSeq const subscriptionId = `runtime-client-events-${connectionId ?? 'inproc'}-${seq}` diff --git a/src/shared/terminal-output-side-effects.ts b/src/shared/terminal-output-side-effects.ts index cde876882..a022e2469 100644 --- a/src/shared/terminal-output-side-effects.ts +++ b/src/shared/terminal-output-side-effects.ts @@ -98,6 +98,8 @@ export type TerminalTitleTracker = { * Titles are unaffected; un-suppressing resets the scanners' cross-chunk carry. */ setTransientFactScanningSuppressed: (suppressed: boolean) => void + /** Enable consumer-only bell, PR-link, and mode-2031 scans without resetting title state. */ + setTransientSideEffectScanningEnabled: (enabled: boolean) => void /** Cancel the stale-title timer and clear accumulated tracker state. */ dispose: () => void } @@ -117,12 +119,13 @@ export function createTerminalTitleTracker( onMode2031Subscribe, onMode2031Unsubscribe } = callbacks - const bellDetector = onBell ? createBellDetector() : null + let bellDetector = onBell ? createBellDetector() : null // Why: created only when a consumer exists so headless serve never pays the per-chunk 133/URL scans. const commandFinishedScanner = onCommandFinished ? createOsc133CommandFinishedScanner(onCommandFinished) : null let prLinkDetector = onPrLink ? createTerminalGitHubPRLinkDetector() : null + let transientSideEffectScanningEnabled = true let transientFactScanningSuppressed = false let mode2031ReplyScanState = INITIAL_MODE_2031_REPLY_SCAN_STATE // Why: seed both so a mid-session tracker behaves as if it had observed the pane's last live title (renderer parity). @@ -210,7 +213,7 @@ export function createTerminalTitleTracker( onPrLink?.(link) } } - if (onMode2031Subscribe || onMode2031Unsubscribe) { + if (transientSideEffectScanningEnabled && (onMode2031Subscribe || onMode2031Unsubscribe)) { const previousMode2031ReplyScanState = options.mode2031PendingSubscribe ? { ...mode2031ReplyScanState, pendingSubscribe: true } : mode2031ReplyScanState @@ -238,7 +241,11 @@ export function createTerminalTitleTracker( } } // The permission BEL rides outside the OSC title; a FRESH detector avoids touching the chunk detector's cross-chunk escape state. - if (onBell && createBellDetector().chunkContainsBell(frame)) { + if ( + transientSideEffectScanningEnabled && + onBell && + createBellDetector().chunkContainsBell(frame) + ) { onBell() } // Why: deliberately skip the 133/PR-link/2031 scanners — fabricated bytes contain none and must not perturb their cross-chunk carry. @@ -271,6 +278,16 @@ export function createTerminalTitleTracker( } } }, + setTransientSideEffectScanningEnabled(enabled: boolean): void { + if (enabled === transientSideEffectScanningEnabled) { + return + } + transientSideEffectScanningEnabled = enabled + bellDetector?.reset() + bellDetector = enabled && onBell ? createBellDetector() : null + prLinkDetector = enabled && onPrLink ? createTerminalGitHubPRLinkDetector() : null + mode2031ReplyScanState = INITIAL_MODE_2031_REPLY_SCAN_STATE + }, dispose(): void { clearStaleTitleTimer() agentTracker?.reset()