diff --git a/src/main/index.ts b/src/main/index.ts index 0310c11286f..9f722f14663 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -129,6 +129,7 @@ import { type SyntheticTitleSpinnerEntry } from './synthetic-title-spinner' import { shouldSendSyntheticTitleFrame } from './synthetic-title-visibility' +import { shouldCopySyntheticTitleFrameToPtyData } from './synthetic-title-frame-routing' import { isCrashReportReason } from '../shared/crash-reporting' import { getSyntheticAgentTitleProfile, @@ -990,10 +991,14 @@ function sendSyntheticTitle(ptyId: string, data: string, options: { force?: bool // Why: feed the per-PTY tracker directly (never onPtyData — emulator state, // tails, transcripts, and stats must not see fabricated bytes) so synthetic // titles/BELs reach pty:sideEffect consumers when main holds side-effect - // authority. The legacy pty:data copy below stays until slice 3 so renderer - // byte parsers keep working while the kill switch is off. + // authority. runtime?.ingestSyntheticTitleFrame(ptyId, data) - mainWindow.webContents.send('pty:data', { id: ptyId, data }) + // Why: only the kill-switch-off renderer still byte-parses synthetic frames; + // under main authority the copy would just mint phantom ACKs for unmetered + // bytes (see synthetic-title-frame-routing.ts). + if (shouldCopySyntheticTitleFrameToPtyData(store?.getSettings())) { + mainWindow.webContents.send('pty:data', { id: ptyId, data }) + } } function isSyntheticTitleWindowVisible(): boolean { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 113a351e680..e970a0606d7 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -3763,6 +3763,97 @@ describe('OrcaRuntimeService', () => { expect(batches[0].facts.at(-1)).toEqual({ kind: 'bell' }) }) + it('emits command-finished facts with best-effort exit codes across chunk splits', () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + + runtime.onPtyData('pty-1', 'output\x1b]133;D;13', 100) + expect(batches).toEqual([]) + runtime.onPtyData('pty-1', '0\x07prompt $ ', 101) + runtime.onPtyData('pty-1', '\x1b]133;D\x07', 102) + + expect(batches.flatMap((batch) => batch.facts)).toEqual([ + { kind: 'command-finished', exitCode: 130 }, + { kind: 'command-finished', exitCode: null } + ]) + }) + + it('emits pr-link facts once per URL with batch attribution', () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + + runtime.onPtyData('pty-1', 'PR https://github.com/acme/orca/pull/4', 100) + runtime.onPtyData('pty-1', '2\r\nand https://github.com/acme/orca/pull/43 done\r\n', 101) + // Repeated URL: deduped per PTY, like the renderer byte detector. + runtime.onPtyData('pty-1', 'again https://github.com/acme/orca/pull/42\r\n', 102) + + expect(batches).toHaveLength(1) + expect(batches[0]).toMatchObject({ + ptyId: 'pty-1', + worktreeId: TEST_WORKTREE_ID, + tabId: 'tab-1' + }) + expect(batches[0].facts).toEqual([ + { + kind: 'pr-link', + link: { + url: 'https://github.com/acme/orca/pull/42', + slug: { owner: 'acme', repo: 'orca' }, + number: 42 + } + }, + { + kind: 'pr-link', + link: { + url: 'https://github.com/acme/orca/pull/43', + slug: { owner: 'acme', repo: 'orca' }, + number: 43 + } + } + ]) + }) + + it('prefers the tracked title over the renderer snapshot lastTitle', async () => { + const { runtime } = createSideEffectRuntime() + const serializeBuffer = vi.fn().mockResolvedValue({ + data: 'visible content', + cols: 80, + rows: 24, + // The renderer xterm never saw the synthetic frame (it no longer + // rides pty:data), so its serializer reports a stale title. + lastTitle: 'stale shell title' + }) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + serializeBuffer, + hasRendererSerializer: () => true, + getSize: () => ({ cols: 80, rows: 24 }) + }) + syncSinglePty(runtime) + + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;⠋ Cursor Agent\x07') + + const snapshot = await runtime.serializeTerminalBuffer('pty-1', { scrollbackRows: 10 }) + expect(snapshot?.source).toBe('renderer') + expect(snapshot?.lastTitle).toBe('⠋ Cursor Agent') + }) + + it('prefers the tracked title over the headless emulator lastTitle', async () => { + const { runtime } = createSideEffectRuntime() + syncSinglePty(runtime) + + runtime.onPtyData('pty-1', '\x1b]0;Codex working\x07real output\r\n', 100) + // The hook-driven idle frame lands only in main's tracker — the + // emulator never sees fabricated bytes (invariant 5). + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + + const snapshot = await runtime.serializeMainTerminalBuffer('pty-1', { scrollbackRows: 10 }) + expect(snapshot?.source).toBe('headless') + expect(snapshot?.lastTitle).toBe('Codex ready') + }) + it('returns a title-only replay snapshot and never historical attention', () => { const { runtime } = createSideEffectRuntime() syncSinglePty(runtime) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index f3b29600f44..d735764e008 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -17,6 +17,7 @@ import type { TerminalSideEffectBatch, TerminalSideEffectFact } from '../../shared/terminal-side-effect-facts' +import type { TerminalGitHubPRLink } from '../../shared/terminal-github-pr-link-detector' import { AGENT_STATUS_STALE_AFTER_MS, type ParsedAgentStatusPayload, @@ -3571,6 +3572,33 @@ export class OrcaRuntimeService { } } + /** Raw last title from main's tracked PTY/leaf records — the title surface + * the tracker (live bytes + synthetic frames) keeps current. */ + private getTrackedRawTitleForPty(ptyId: string): string | null { + const recordTitle = this.ptysById.get(ptyId)?.lastOscTitle + if (recordTitle) { + return recordTitle + } + for (const leaf of this.getLeavesForPty(ptyId)) { + if (leaf.lastOscTitle) { + return leaf.lastOscTitle + } + } + return null + } + + /** Why: synthetic agent title frames no longer ride pty:data, so neither + * renderer xterm nor the headless emulator observes them. Mobile-parity + * snapshot titles must prefer main's tracker over snapshot lastTitle, or + * hook-driven spinner/idle titles vanish from mobile tabs. */ + private preferTrackedLastTitle(ptyId: string, snapshot: T): T { + const tracked = this.getTrackedRawTitleForPty(ptyId) + if (!tracked) { + return snapshot + } + return { ...snapshot, lastTitle: tracked } + } + /** Decorative comparison key: spinner frame glyphs stripped, derived agent * status kept so a working→idle flip with an otherwise-equal label still * counts as a change. */ @@ -3646,12 +3674,19 @@ export class OrcaRuntimeService { onAgentExited: () => { this.recordTerminalSideEffectFact(ptyId, { kind: 'agent-exited' }) }, - // Why: bell facts exist only for the pty:sideEffect channel. Headless - // serve has no consumer, so skip the per-chunk bell walk entirely. + // Why: bell/command-finished/pr-link facts exist only for the + // pty:sideEffect channel. Headless serve has no consumer, so skip the + // per-chunk bell walk and 133/URL scans entirely. ...(this.onTerminalSideEffects ? { onBell: () => { this.recordTerminalSideEffectFact(ptyId, { kind: 'bell' }) + }, + onCommandFinished: (exitCode: number | null) => { + this.recordTerminalSideEffectFact(ptyId, { kind: 'command-finished', exitCode }) + }, + onPrLink: (link: TerminalGitHubPRLink) => { + this.recordTerminalSideEffectFact(ptyId, { kind: 'pr-link', link }) } } : {}) @@ -4124,10 +4159,9 @@ export class OrcaRuntimeService { // If renderer serialization races reload/unmount, the runtime snapshot // below can still preserve colored terminal state. } - if (rendererSnapshot && rendererSnapshot.data.length > 0) { - return { ...rendererSnapshot, source: 'renderer' } - } - return rendererSnapshot ? { ...rendererSnapshot, source: 'renderer' } : null + return rendererSnapshot + ? this.preferTrackedLastTitle(ptyId, { ...rendererSnapshot, source: 'renderer' as const }) + : null } private async withVisibleSnapshotFallback( @@ -4212,15 +4246,15 @@ export class OrcaRuntimeService { const snapshot = state.emulator.getSnapshot({ scrollbackRows }) const data = snapshot.rehydrateSequences + snapshot.snapshotAnsi return data.length > 0 || opts.includeEmpty === true - ? { + ? this.preferTrackedLastTitle(ptyId, { data, cols: snapshot.cols, rows: snapshot.rows, cwd: snapshot.cwd, lastTitle: snapshot.lastTitle, seq: state.outputSequence, - source: 'headless' - } + source: 'headless' as const + }) : null } diff --git a/src/main/synthetic-title-frame-routing.test.ts b/src/main/synthetic-title-frame-routing.test.ts new file mode 100644 index 00000000000..b4bbf35ecd2 --- /dev/null +++ b/src/main/synthetic-title-frame-routing.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' +import { shouldCopySyntheticTitleFrameToPtyData } from './synthetic-title-frame-routing' + +describe('shouldCopySyntheticTitleFrameToPtyData', () => { + it('keeps the legacy pty:data copy only while the kill switch is off', () => { + // Authority off: renderer byte parsers are the sole synthetic-frame + // consumer, so the legacy copy must keep flowing. + expect(shouldCopySyntheticTitleFrameToPtyData({ terminalMainSideEffectAuthority: false })).toBe( + true + ) + }) + + it('skips the copy under main authority — tracker ingest is the only consumer', () => { + // Why: under authority the copy would only mint phantom renderer ACKs + // for fabricated bytes main never metered. + expect(shouldCopySyntheticTitleFrameToPtyData({ terminalMainSideEffectAuthority: true })).toBe( + false + ) + // Default-on: an unset switch means main authority. + expect(shouldCopySyntheticTitleFrameToPtyData({})).toBe(false) + expect(shouldCopySyntheticTitleFrameToPtyData(null)).toBe(false) + expect(shouldCopySyntheticTitleFrameToPtyData(undefined)).toBe(false) + }) +}) diff --git a/src/main/synthetic-title-frame-routing.ts b/src/main/synthetic-title-frame-routing.ts new file mode 100644 index 00000000000..3bb10a030fa --- /dev/null +++ b/src/main/synthetic-title-frame-routing.ts @@ -0,0 +1,14 @@ +import type { GlobalSettings } from '../shared/types' + +/** + * Why: with the side-effect kill switch off, renderer byte parsers are the + * ONLY consumer of main-fabricated OSC title frames, so they must still ride + * `pty:data`. With main authority on (the default), the tracker ingest is the + * sole consumer and the legacy copy would only mint phantom renderer ACKs for + * bytes main never metered. See terminal-side-effect-authority.md (slice 3). + */ +export function shouldCopySyntheticTitleFrameToPtyData( + settings: Pick | null | undefined +): boolean { + return settings?.terminalMainSideEffectAuthority === false +} diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts index 6f222088adb..1cb9b0018cc 100644 --- a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts +++ b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts @@ -468,8 +468,8 @@ describe('startParkedTerminalByteWatcher', () => { // // With the kill switch on, the watcher must not register byte parsers — // main is the single byte parser and the watcher's policy block consumes - // pty:sideEffect facts instead. The byte sidecar stays only for the 2031 - // reply and PR-link scan (they move to main in a later slice). + // pty:sideEffect facts instead. The byte sidecar stays ONLY for the 2031 + // reply (query authority never moves to main); PR links arrive as facts. describe('with main side-effect authority on', () => { function enableMainAuthority(): void { mockStoreState.settings = { @@ -664,15 +664,36 @@ describe('startParkedTerminalByteWatcher', () => { dispose() }) - it('still answers DECSET 2031 and observes PR links from the byte sidecar', async () => { + it('keeps the byte sidecar only for the DECSET 2031 reply — no PR byte scan', async () => { enableMainAuthority() const { dispose, sendInput } = await startWatcher() emit('\x1b[?2031h') expect(sendInput).toHaveBeenCalledWith('\x1b[?997;1n') + // Why: pr-link facts arrive on the channel; byte-scanning here too + // would observe every link twice. emit('PR: https://github.com/orca-dev/orca/pull/42\r\n') + expect(mockStoreState.observeTerminalGitHubPullRequestLink).not.toHaveBeenCalled() + dispose() + }) + + it('observes PR links from pr-link facts with worktree attribution', async () => { + enableMainAuthority() + const { dispose } = await startWatcher() + + const link = { + url: 'https://github.com/orca-dev/orca/pull/421', + slug: { owner: 'orca-dev', repo: 'orca' }, + number: 421 + } + await dispatchFacts([{ kind: 'pr-link', link }]) + expect(mockStoreState.observeTerminalGitHubPullRequestLink).toHaveBeenCalledTimes(1) + expect(mockStoreState.observeTerminalGitHubPullRequestLink).toHaveBeenCalledWith( + WORKTREE_ID, + link + ) dispose() }) diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts index 7fbf1f8d2c3..ece2b160c13 100644 --- a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts +++ b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts @@ -19,7 +19,7 @@ import { } from '../../../../shared/terminal-color-scheme-protocol' import { useAppStore } from '@/store' import { getSystemPrefersDark } from '@/lib/terminal-theme' -import { createTerminalGitHubPRLinkDetector } from '@/lib/terminal-github-pr-link-detector' +import { createTerminalGitHubPRLinkDetector } from '../../../../shared/terminal-github-pr-link-detector' import { AGENT_TASK_COMPLETE_NOTIFICATION_GRACE_MS, isAgentTaskCompleteOsNotificationEnabledFromState, @@ -93,7 +93,6 @@ export function startParkedTerminalByteWatcher( let bellNotificationTimer: ReturnType | null = null let agentTaskCompleteTimer: ReturnType | null = null let mode2031ScanTail = '' - const observeTerminalGitHubPRLink = createTerminalGitHubPRLinkDetector() const clearBellNotificationTimer = (): void => { if (bellNotificationTimer !== null) { @@ -238,12 +237,21 @@ export function startParkedTerminalByteWatcher( ...(options.initialTitle !== undefined ? { initialAgentTitle: options.initialTitle } : {}), ...sideEffectCallbacks }) + // Why (byte-parser mode only): with main authority, pr-link facts arrive on + // the channel below; byte-scanning too would observe every link twice. + const observeTerminalGitHubPRLink = mainSideEffectAuthority + ? null + : createTerminalGitHubPRLinkDetector() const unregisterFactConsumer = mainSideEffectAuthority ? registerTerminalSideEffectFactConsumer({ ptyId, // Why: no title snapshot on park — the pane's runtime title slot is // already current at park time, exactly like the byte-parser mode. - callbacks: sideEffectCallbacks + callbacks: { + ...sideEffectCallbacks, + onPrLink: (link) => + useAppStore.getState().observeTerminalGitHubPullRequestLink(worktreeId, link) + } }) : null @@ -260,16 +268,20 @@ export function startParkedTerminalByteWatcher( sendInput(mode2031SequenceFor(resolveTerminalColorSchemeMode(settings, getSystemPrefersDark()))) } - // Why: the byte sidecar stays in BOTH modes for the 2031 reply and PR-link - // scan — those move to main in a later slice. Only the title/bell/agent - // parsing is gated: processor is null when main holds authority. + // Why: under main authority the byte sidecar stays ONLY for the DECSET 2031 + // reply — query authority belongs to the view/watcher (model/view contract + // invariant 6), so it can never move to main. Title/bell/agent parsing and + // the PR-link scan are byte-parser-mode only (null when main holds + // authority; their facts arrive on pty:sideEffect instead). const unsubscribe = subscribeToPtyData(ptyId, (data) => { // Why: empty pane callbacks — the watcher wants only the parser side // effects; there is no xterm to deliver bytes to. processor?.processData(data, {}) respondToMode2031Subscribe(data) - for (const link of observeTerminalGitHubPRLink(data)) { - useAppStore.getState().observeTerminalGitHubPullRequestLink(worktreeId, link) + if (observeTerminalGitHubPRLink) { + for (const link of observeTerminalGitHubPRLink(data)) { + useAppStore.getState().observeTerminalGitHubPullRequestLink(worktreeId, link) + } } }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 260ac4983d8..ee39fd85238 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -5281,6 +5281,100 @@ describe('connectPanePty', () => { expect(deps.markTerminalPaneUnread).not.toHaveBeenCalled() }) + it('drops the agent status from a command-finished fact like the byte path did', async () => { + enableMainAuthority() + const { connectPanePty } = await import('./pty-connection') + const handler = await import('./terminal-side-effect-facts-handler') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey = { + [paneKey]: { + paneKey, + state: 'done', + prompt: 'hi', + updatedAt: 1000, + stateStartedAt: 1000, + agentType: 'codex', + stateHistory: [] + } + } + + connectPanePty(createPane(1) as never, createManager(1) as never, createDeps() as never) + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as (ptyId: string) => void + onPtySpawn('pty-fact-133') + + handler._dispatchTerminalSideEffectBatchForTest({ + ptyId: 'pty-fact-133', + seq: 1, + facts: [{ kind: 'command-finished', exitCode: 130 }] + }) + + expect(mockStoreState.dropAgentStatus).toHaveBeenCalledWith(paneKey) + expect(mockStoreState.removeAgentStatus).not.toHaveBeenCalled() + }) + + it('routes pr-link facts to the worktree PR observer', async () => { + enableMainAuthority() + const { connectPanePty } = await import('./pty-connection') + const handler = await import('./terminal-side-effect-facts-handler') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + + connectPanePty(createPane(1) as never, createManager(1) as never, createDeps() as never) + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as (ptyId: string) => void + onPtySpawn('pty-fact-pr') + + const link = { + url: 'https://github.com/acme/orca/pull/42', + slug: { owner: 'acme', repo: 'orca' }, + number: 42 + } + handler._dispatchTerminalSideEffectBatchForTest({ + ptyId: 'pty-fact-pr', + seq: 1, + facts: [{ kind: 'pr-link', link }] + }) + + expect(mockStoreState.observeTerminalGitHubPullRequestLink).toHaveBeenCalledWith('wt-1', link) + }) + + it('does not byte-scan PR links or OSC 133 — facts are the only consumer', async () => { + enableMainAuthority() + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-authority-bytes' + } + ) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey = { + [paneKey]: { + paneKey, + state: 'done', + prompt: 'hi', + updatedAt: 1000, + stateStartedAt: 1000, + agentType: 'codex', + stateHistory: [] + } + } + + connectPanePty(createPane(1) as never, createManager(1) as never, createDeps() as never) + await flushAsyncTicks() + expect(capturedDataCallback.current).not.toBeNull() + + capturedDataCallback.current?.('Created https://github.com/acme/orca/pull/42\r\n') + capturedDataCallback.current?.('\x1b]133;D;130\x07prompt $ ') + + expect(mockStoreState.observeTerminalGitHubPullRequestLink).not.toHaveBeenCalled() + expect(mockStoreState.dropAgentStatus).not.toHaveBeenCalled() + }) + it('honors the persisted kill switch for panes bound before settings hydrate', async () => { // Pre-hydration: the store has no settings yet, but the user persisted // the kill switch off. The pane must register byte parsers, not a fact diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 49c7e4e467e..733c1878486 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -75,7 +75,7 @@ import { } from './terminal-bracketed-paste' import { createCommandCodeOutputStatusDetector } from './command-code-output-status' import type { PtyDataMeta } from './pty-dispatcher' -import { createTerminalGitHubPRLinkDetector } from '@/lib/terminal-github-pr-link-detector' +import { createTerminalGitHubPRLinkDetector } from '../../../../shared/terminal-github-pr-link-detector' import { installConptyDeviceAttributesHandler } from './terminal-conpty-device-attributes' import { cancelScheduledHiddenOutputRestore, @@ -889,33 +889,40 @@ export function connectPanePty( } return pendingWrite.then(() => interruptInference.flushPending()) } - const commandLifecycle = createTerminalCommandLifecycle({ - onCommandFinished: () => { - const state = useAppStore.getState() - const entry = state.agentStatusByPaneKey[cacheKey] - const inferenceResult = flushPendingInterruptInference() - if (inferenceResult === true) { - // Why: OSC 133 D means the foreground shell command exited. If an - // interrupt was inferred first, drop only when the current interrupted - // row is still the same turn; otherwise a killed OpenCode CLI leaves a - // stale "interrupted" row even though the process is gone. - dropCommandFinishedStatusIfSameTurn(entry, { allowInferredInterrupt: true }) - return - } - if (inferenceResult instanceof Promise) { - void inferenceResult.then((applied) => { - dropCommandFinishedStatusIfSameTurn(entry, { - allowInferredInterrupt: applied === true - }) - }) - return - } - // Why: OSC 133 D marks the foreground shell command exiting. Remove the - // row without retaining a done snapshot; this section represents a live - // agent process, and the shell prompt means that process is gone. - dropCommandFinishedStatusIfSameTurn(entry) + // Why: one command-finished policy whether the signal arrives as bytes + // (remote PTYs, kill switch off) or as a main-derived pty:sideEffect fact — + // routing both through this handler keeps the drop/interrupt semantics + // identical across authority modes. + const handleCommandFinished = (_bestEffortExitCode: number | null): void => { + const state = useAppStore.getState() + const entry = state.agentStatusByPaneKey[cacheKey] + const inferenceResult = flushPendingInterruptInference() + if (inferenceResult === true) { + // Why: OSC 133 D means the foreground shell command exited. If an + // interrupt was inferred first, drop only when the current interrupted + // row is still the same turn; otherwise a killed OpenCode CLI leaves a + // stale "interrupted" row even though the process is gone. + dropCommandFinishedStatusIfSameTurn(entry, { allowInferredInterrupt: true }) + return } + if (inferenceResult instanceof Promise) { + void inferenceResult.then((applied) => { + dropCommandFinishedStatusIfSameTurn(entry, { + allowInferredInterrupt: applied === true + }) + }) + return + } + // Why: OSC 133 D marks the foreground shell command exiting. Remove the + // row without retaining a done snapshot; this section represents a live + // agent process, and the shell prompt means that process is gone. + dropCommandFinishedStatusIfSameTurn(entry) + } + const commandLifecycle = createTerminalCommandLifecycle({ + onCommandFinished: handleCommandFinished }) + // Why: the xterm OSC 133 swallow is rendering hygiene, not a side effect — + // it stays attached in every authority mode. commandLifecycle.attachXtermConsumer(pane.terminal) const onTerminalKeyDown = (event: KeyboardEvent): void => { if (isPlainEscapeKeyEvent(event)) { @@ -993,7 +1000,10 @@ export function connectPanePty( onBell, onAgentBecameIdle, onAgentBecameWorking, - onAgentExited + onAgentExited, + onCommandFinished: handleCommandFinished, + onPrLink: (link) => + useAppStore.getState().observeTerminalGitHubPullRequestLink(deps.worktreeId, link) }, restoreTitleOnRegister: true }) @@ -2660,11 +2670,17 @@ export function connectPanePty( const dataCallback = (data: string, meta?: PtyDataMeta): void => { resetHiddenOutputRestoreIfPtyChanged() observeTerminalBracketedPasteModeOutput(pane.terminal, data) - for (const link of observeTerminalGitHubPRLink(data)) { - useAppStore.getState().observeTerminalGitHubPullRequestLink(deps.worktreeId, link) + // Why: with main side-effect authority, command-finished and pr-link + // arrive as pty:sideEffect facts — byte-scanning here too would + // double-fire the same policy. Remote-runtime PTYs (and the kill + // switch off) keep this byte path as their only parser. + if (!mainSideEffectAuthority) { + for (const link of observeTerminalGitHubPRLink(data)) { + useAppStore.getState().observeTerminalGitHubPullRequestLink(deps.worktreeId, link) + } + commandLifecycle.handlePtyData(data) } commandCodeOutputStatusDetector.observe(data) - commandLifecycle.handlePtyData(data) // Why: split-pane layouts have multiple visible-but-inactive panes whose // output the user is watching. Throttle only when the pane or whole // Electron document is hidden. diff --git a/src/renderer/src/components/terminal-pane/terminal-command-lifecycle.ts b/src/renderer/src/components/terminal-pane/terminal-command-lifecycle.ts index 3fb103efeac..69b6d923423 100644 --- a/src/renderer/src/components/terminal-pane/terminal-command-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/terminal-command-lifecycle.ts @@ -1,99 +1,32 @@ import type { Terminal, IDisposable } from '@xterm/xterm' +import { createOsc133CommandFinishedScanner } from '../../../../shared/terminal-osc133-command-finished' type TerminalCommandLifecycleOptions = { onCommandFinished: (bestEffortExitCode: number | null) => void } -type OscTerminator = { - index: number - length: number -} - -const OSC_133_PREFIX = '\x1b]133;' -const MAX_OSC_CARRY_LENGTH = 4096 - -function findOscTerminator(data: string, startIndex: number): OscTerminator | null { - const bel = data.indexOf('\x07', startIndex) - const st = data.indexOf('\x1b\\', startIndex) - - if (bel === -1 && st === -1) { - return null - } - if (bel !== -1 && (st === -1 || bel < st)) { - return { index: bel, length: 1 } - } - return { index: st, length: 2 } -} - -function parseBestEffortExitCode(value: string | undefined): number | null { - if (!value) { - return null - } - const parsed = Number.parseInt(value, 10) - return Number.isNaN(parsed) ? null : parsed -} - -function findPrefixCarry(data: string): string { - const maxCarryLength = Math.min(data.length, OSC_133_PREFIX.length - 1) - for (let length = maxCarryLength; length > 0; length -= 1) { - const suffix = data.slice(data.length - length) - if (OSC_133_PREFIX.startsWith(suffix)) { - return suffix - } - } - return '' -} - export function createTerminalCommandLifecycle(options: TerminalCommandLifecycleOptions): { handlePtyData: (data: string) => void attachXtermConsumer: (terminal: Terminal) => IDisposable dispose: () => void } { - let carry = '' + // Why: the byte parsing lives in shared so main's side-effect tracker emits + // identical command-finished facts for local/SSH PTYs; this renderer wrapper + // remains the byte path for remote-runtime PTYs and the kill-switch-off mode. + const scanner = createOsc133CommandFinishedScanner(options.onCommandFinished) const disposables: IDisposable[] = [] - const handleOsc133 = (payload: string): void => { - const [sequence, exitCode] = payload.split(';') - if (sequence === 'D') { - options.onCommandFinished(parseBestEffortExitCode(exitCode)) - } - } - - const handlePtyData = (data: string): void => { - let combined = carry + data - carry = '' - - while (combined.length > 0) { - const start = combined.indexOf(OSC_133_PREFIX) - if (start === -1) { - carry = findPrefixCarry(combined) - return - } - - const payloadStart = start + OSC_133_PREFIX.length - const terminator = findOscTerminator(combined, payloadStart) - if (!terminator) { - carry = combined.slice(start) - if (carry.length > MAX_OSC_CARRY_LENGTH) { - carry = carry.slice(carry.length - MAX_OSC_CARRY_LENGTH) - } - return - } - - handleOsc133(combined.slice(payloadStart, terminator.index)) - combined = combined.slice(terminator.index + terminator.length) - } - } - return { - handlePtyData, + handlePtyData: scanner.scan, attachXtermConsumer(terminal) { + // Why: swallow OSC 133 so shell-integration markers never paint — + // rendering hygiene that applies regardless of side-effect authority. const disposable = terminal.parser.registerOscHandler(133, () => true) disposables.push(disposable) return disposable }, dispose() { - carry = '' + scanner.reset() for (const disposable of disposables.splice(0)) { disposable.dispose() } diff --git a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts index 395eb87d732..074fd7cad98 100644 --- a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts @@ -173,6 +173,72 @@ describe('registerTerminalSideEffectFactConsumer', () => { ]) }) + it('routes command-finished and pr-link facts to the registered consumer', () => { + const events: unknown[][] = [] + registerTerminalSideEffectFactConsumer({ + ptyId: PTY_ID, + callbacks: { + onCommandFinished: (exitCode) => events.push(['finished', exitCode]), + onPrLink: (link) => events.push(['pr', link.url, link.number]) + } + }) + + _dispatchTerminalSideEffectBatchForTest( + batch([ + { kind: 'command-finished', exitCode: 130 }, + { + kind: 'pr-link', + link: { + url: 'https://github.com/acme/orca/pull/42', + slug: { owner: 'acme', repo: 'orca' }, + number: 42 + } + }, + { kind: 'command-finished', exitCode: null } + ]) + ) + + expect(events).toEqual([ + ['finished', 130], + ['pr', 'https://github.com/acme/orca/pull/42', 42], + ['finished', null] + ]) + }) + + it('never replays command-finished or pr-link facts', () => { + // Why: like bells and agent transitions, command/PR facts are attention + // signals — replay snapshots restore title state only. + const events: unknown[][] = [] + registerTerminalSideEffectFactConsumer({ + ptyId: PTY_ID, + callbacks: { + onTitleChange: (normalizedTitle) => events.push(['title', normalizedTitle]), + onCommandFinished: (exitCode) => events.push(['finished', exitCode]), + onPrLink: (link) => events.push(['pr', link.url]) + } + }) + + _dispatchTerminalSideEffectBatchForTest( + batch( + [ + { kind: 'title', normalizedTitle: 'restored', rawTitle: 'restored' }, + { kind: 'command-finished', exitCode: 0 }, + { + kind: 'pr-link', + link: { + url: 'https://github.com/acme/orca/pull/42', + slug: { owner: 'acme', repo: 'orca' }, + number: 42 + } + } + ], + { replay: true, seq: 5 } + ) + ) + + expect(events).toEqual([['title', 'restored']]) + }) + it('passes stale-clear provenance through to the title and idle callbacks', () => { const events: unknown[][] = [] registerTerminalSideEffectFactConsumer({ diff --git a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts index 7c9789b69bf..6774bd9a8bf 100644 --- a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts +++ b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts @@ -12,6 +12,7 @@ * where pre-mount output produces no attention side effects. */ import type { GlobalSettings } from '../../../../shared/types' +import type { TerminalGitHubPRLink } from '../../../../shared/terminal-github-pr-link-detector' import type { TerminalSideEffectBatch, TerminalSideEffectFact @@ -73,6 +74,10 @@ export type TerminalSideEffectFactConsumerCallbacks = { onAgentBecameIdle?: (title: string, meta?: { staleWorkingTitleClear?: boolean }) => void onAgentBecameWorking?: () => void onAgentExited?: () => void + /** OSC 133;D — same policy hook the byte-mode commandLifecycle drove + * (stale agent-status row drop + interrupt-inference coordination). */ + onCommandFinished?: (bestEffortExitCode: number | null) => void + onPrLink?: (link: TerminalGitHubPRLink) => void } type ConsumerEntry = { @@ -109,6 +114,12 @@ function applyLiveFact(entry: ConsumerEntry, fact: TerminalSideEffectFact, seq: return case 'agent-exited': entry.callbacks.onAgentExited?.() + return + case 'command-finished': + entry.callbacks.onCommandFinished?.(fact.exitCode) + return + case 'pr-link': + entry.callbacks.onPrLink?.(fact.link) } } 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 6c4f2f10fbc..b505486278a 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 @@ -8,8 +8,10 @@ // event sequences match. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createAgentStatusOscProcessor } from '../../../../shared/agent-status-osc' +import { createTerminalGitHubPRLinkDetector } from '../../../../shared/terminal-github-pr-link-detector' import { createTerminalTitleTracker } from '../../../../shared/terminal-output-side-effects' import { createPtyOutputProcessor } from './pty-transport' +import { createTerminalCommandLifecycle } from './terminal-command-lifecycle' const ESC = '\x1b' const BEL = '\x07' @@ -69,7 +71,9 @@ function createMainPath(): TitleFactPath { } } -function feedBoth(paths: { renderer: TitleFactPath; main: TitleFactPath }, chunk: string): void { +type ChunkFeed = { feed: (chunk: string) => void } + +function feedBoth(paths: { renderer: ChunkFeed; main: ChunkFeed }, chunk: string): void { paths.renderer.feed(chunk) paths.main.feed(chunk) } @@ -187,3 +191,95 @@ describe('main title tracker parity with the renderer transport processor', () = expect(paths.main.events).toEqual([]) }) }) + +// Why: slice 3 moves the renderer's OSC 133;D and PR-link byte parsing into +// main's tracker for local/SSH PTYs. Both must derive identical fact +// sequences from the same chunk boundaries, or flipping the kill switch +// changes which commands/links are observed. +type LifecycleFactEvent = ['command-finished', number | null] | ['pr-link', string, number] + +type LifecycleFactPath = { + events: LifecycleFactEvent[] + feed: (chunk: string) => void +} + +function createRendererLifecyclePath(): LifecycleFactPath { + const events: LifecycleFactEvent[] = [] + // Why: mirrors pty-connection's dataCallback wiring — the transport + // processor strips OSC 9999 before the lifecycle/PR-link byte scans run. + const processAgentStatusChunk = createAgentStatusOscProcessor() + const lifecycle = createTerminalCommandLifecycle({ + onCommandFinished: (exitCode) => events.push(['command-finished', exitCode]) + }) + const detectPRLinks = createTerminalGitHubPRLinkDetector() + return { + events, + feed(chunk: string): void { + const clean = processAgentStatusChunk(chunk).cleanData + lifecycle.handlePtyData(clean) + for (const link of detectPRLinks(clean)) { + events.push(['pr-link', link.url, link.number]) + } + } + } +} + +function createMainLifecyclePath(): LifecycleFactPath { + const events: LifecycleFactEvent[] = [] + const processAgentStatusChunk = createAgentStatusOscProcessor() + const tracker = createTerminalTitleTracker({ + onCommandFinished: (exitCode) => events.push(['command-finished', exitCode]), + onPrLink: (link) => events.push(['pr-link', link.url, link.number]) + }) + return { + events, + feed(chunk: string): void { + tracker.handleChunk(processAgentStatusChunk(chunk).cleanData) + } + } +} + +describe('main tracker parity with renderer 133;D and PR-link byte parsers', () => { + let paths: { renderer: LifecycleFactPath; main: LifecycleFactPath } + + beforeEach(() => { + paths = { renderer: createRendererLifecyclePath(), main: createMainLifecyclePath() } + }) + + it('derives identical command-finished facts from split OSC 133;D chunks', () => { + feedBoth(paths, `output${ESC}]133`) + feedBoth(paths, ';D;13') + feedBoth(paths, `0${BEL}prompt $ `) + feedBoth(paths, `${ESC}]133;D;0${BEL}`) + feedBoth(paths, `${ESC}]133;D${ST}`) + + expect(paths.main.events).toEqual(paths.renderer.events) + expect(paths.main.events).toEqual([ + ['command-finished', 130], + ['command-finished', 0], + ['command-finished', null] + ]) + }) + + it('derives identical pr-link facts from split and repeated URLs', () => { + feedBoth(paths, 'Created https://github.com/acme/orca/pull/4') + feedBoth(paths, '2\r\nAlso https://github.com/acme/orca/pull/43 merged\r\n') + feedBoth(paths, 'again https://github.com/acme/orca/pull/42\r\n') + + expect(paths.main.events).toEqual(paths.renderer.events) + expect(paths.main.events).toEqual([ + ['pr-link', 'https://github.com/acme/orca/pull/42', 42], + ['pr-link', 'https://github.com/acme/orca/pull/43', 43] + ]) + }) + + it('ignores 133;D and PR URLs inside stripped OSC 9999 payloads in both paths', () => { + feedBoth( + paths, + `${ESC}]9999;{"state":"done","prompt":"https://github.com/acme/orca/pull/9"}${BEL}\r\n` + ) + + expect(paths.main.events).toEqual(paths.renderer.events) + expect(paths.main.events).toEqual([]) + }) +}) diff --git a/src/renderer/src/lib/github-links.ts b/src/renderer/src/lib/github-links.ts index cc379cc99de..87844dc9d04 100644 --- a/src/renderer/src/lib/github-links.ts +++ b/src/renderer/src/lib/github-links.ts @@ -1,122 +1,4 @@ -const GH_ITEM_PATH_RE = /^\/([^/]+)\/([^/]+)\/(issues|pull)\/(\d+)(?:\/.*)?$/i - -export type RepoSlug = { - owner: string - repo: string -} - -export type GitHubLinkQuery = { - query: string - directNumber: number | null -} - -export function buildGitHubRepoUrl(slug: RepoSlug | null | undefined): string | null { - if (!slug?.owner || !slug.repo) { - return null - } - return `https://github.com/${encodeURIComponent(slug.owner)}/${encodeURIComponent(slug.repo)}` -} - -function matchGitHubItemPath(url: URL): RegExpExecArray | null { - return GH_ITEM_PATH_RE.exec(url.pathname.replace(/\/+$/, '')) -} - -/** - * Parses a GitHub issue/PR reference from plain input. - * Supports issue/PR numbers (e.g. "42"), "#42", and full GitHub URLs. - */ -export function parseGitHubIssueOrPRNumber(input: string): number | null { - const trimmed = input.trim() - if (!trimmed) { - return null - } - - const numeric = trimmed.startsWith('#') ? trimmed.slice(1) : trimmed - if (/^\d+$/.test(numeric)) { - return Number.parseInt(numeric, 10) - } - - let url: URL - try { - url = new URL(trimmed) - } catch { - return null - } - - if (!/^(?:www\.)?github\.com$/i.test(url.hostname)) { - return null - } - - const match = matchGitHubItemPath(url) - if (!match) { - return null - } - - return Number.parseInt(match[4], 10) -} - -/** - * Parses an owner/repo slug plus issue/PR number from a GitHub URL. Returns - * null for anything that isn't a recognizable github.com issue or pull URL. - */ -export function parseGitHubIssueOrPRLink(input: string): { - slug: RepoSlug - number: number - type: 'issue' | 'pr' -} | null { - const trimmed = input.trim() - if (!trimmed) { - return null - } - - let url: URL - try { - url = new URL(trimmed) - } catch { - return null - } - - if (!/^(?:www\.)?github\.com$/i.test(url.hostname)) { - return null - } - - const match = matchGitHubItemPath(url) - if (!match) { - return null - } - - return { - slug: { owner: match[1], repo: match[2] }, - type: match[3].toLowerCase() === 'pull' ? 'pr' : 'issue', - number: Number.parseInt(match[4], 10) - } -} - -/** - * Normalizes link-picker input so both raw issue/PR numbers and full GitHub - * URLs resolve to a usable query + direct-number lookup. - */ -export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { - const trimmed = raw.trim() - if (!trimmed) { - return { query: '', directNumber: null } - } - - const direct = parseGitHubIssueOrPRNumber(trimmed) - if (direct !== null && !trimmed.startsWith('http')) { - return { query: trimmed, directNumber: direct } - } - - const link = parseGitHubIssueOrPRLink(trimmed) - if (!link) { - return { query: trimmed, directNumber: null } - } - - // Why: any github.com issue/pull URL is accepted by number regardless of - // slug, since fork checkouts can legitimately target upstream issues whose - // slug differs from the origin remote. - return { - query: trimmed, - directNumber: link.number - } -} +// Why: the parsing core moved to shared so main's terminal side-effect +// tracker can emit pr-link facts (terminal-side-effect-authority.md, slice 3). +// Re-exported here so renderer consumers keep their '@/lib' import path. +export * from '../../../shared/github-links' diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 4fabef17d84..1ad0ea27c29 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -17,7 +17,7 @@ import type { WorktreeRemoteBranchConflictEvent, WorktreeMeta } from '../../../../shared/types' -import type { TerminalGitHubPRLink } from '@/lib/terminal-github-pr-link-detector' +import type { TerminalGitHubPRLink } from '../../../../shared/terminal-github-pr-link-detector' import type { PendingWorktreeCreation, WorktreeCreationPhase diff --git a/src/shared/github-links.ts b/src/shared/github-links.ts new file mode 100644 index 00000000000..cc379cc99de --- /dev/null +++ b/src/shared/github-links.ts @@ -0,0 +1,122 @@ +const GH_ITEM_PATH_RE = /^\/([^/]+)\/([^/]+)\/(issues|pull)\/(\d+)(?:\/.*)?$/i + +export type RepoSlug = { + owner: string + repo: string +} + +export type GitHubLinkQuery = { + query: string + directNumber: number | null +} + +export function buildGitHubRepoUrl(slug: RepoSlug | null | undefined): string | null { + if (!slug?.owner || !slug.repo) { + return null + } + return `https://github.com/${encodeURIComponent(slug.owner)}/${encodeURIComponent(slug.repo)}` +} + +function matchGitHubItemPath(url: URL): RegExpExecArray | null { + return GH_ITEM_PATH_RE.exec(url.pathname.replace(/\/+$/, '')) +} + +/** + * Parses a GitHub issue/PR reference from plain input. + * Supports issue/PR numbers (e.g. "42"), "#42", and full GitHub URLs. + */ +export function parseGitHubIssueOrPRNumber(input: string): number | null { + const trimmed = input.trim() + if (!trimmed) { + return null + } + + const numeric = trimmed.startsWith('#') ? trimmed.slice(1) : trimmed + if (/^\d+$/.test(numeric)) { + return Number.parseInt(numeric, 10) + } + + let url: URL + try { + url = new URL(trimmed) + } catch { + return null + } + + if (!/^(?:www\.)?github\.com$/i.test(url.hostname)) { + return null + } + + const match = matchGitHubItemPath(url) + if (!match) { + return null + } + + return Number.parseInt(match[4], 10) +} + +/** + * Parses an owner/repo slug plus issue/PR number from a GitHub URL. Returns + * null for anything that isn't a recognizable github.com issue or pull URL. + */ +export function parseGitHubIssueOrPRLink(input: string): { + slug: RepoSlug + number: number + type: 'issue' | 'pr' +} | null { + const trimmed = input.trim() + if (!trimmed) { + return null + } + + let url: URL + try { + url = new URL(trimmed) + } catch { + return null + } + + if (!/^(?:www\.)?github\.com$/i.test(url.hostname)) { + return null + } + + const match = matchGitHubItemPath(url) + if (!match) { + return null + } + + return { + slug: { owner: match[1], repo: match[2] }, + type: match[3].toLowerCase() === 'pull' ? 'pr' : 'issue', + number: Number.parseInt(match[4], 10) + } +} + +/** + * Normalizes link-picker input so both raw issue/PR numbers and full GitHub + * URLs resolve to a usable query + direct-number lookup. + */ +export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { + const trimmed = raw.trim() + if (!trimmed) { + return { query: '', directNumber: null } + } + + const direct = parseGitHubIssueOrPRNumber(trimmed) + if (direct !== null && !trimmed.startsWith('http')) { + return { query: trimmed, directNumber: direct } + } + + const link = parseGitHubIssueOrPRLink(trimmed) + if (!link) { + return { query: trimmed, directNumber: null } + } + + // Why: any github.com issue/pull URL is accepted by number regardless of + // slug, since fork checkouts can legitimately target upstream issues whose + // slug differs from the origin remote. + return { + query: trimmed, + directNumber: link.number + } +} diff --git a/src/renderer/src/lib/terminal-github-pr-link-detector.test.ts b/src/shared/terminal-github-pr-link-detector.test.ts similarity index 100% rename from src/renderer/src/lib/terminal-github-pr-link-detector.test.ts rename to src/shared/terminal-github-pr-link-detector.test.ts diff --git a/src/renderer/src/lib/terminal-github-pr-link-detector.ts b/src/shared/terminal-github-pr-link-detector.ts similarity index 88% rename from src/renderer/src/lib/terminal-github-pr-link-detector.ts rename to src/shared/terminal-github-pr-link-detector.ts index c0b22629951..7c70b60d91e 100644 --- a/src/renderer/src/lib/terminal-github-pr-link-detector.ts +++ b/src/shared/terminal-github-pr-link-detector.ts @@ -1,3 +1,12 @@ +/** + * Chunk-boundary-safe GitHub PR URL scan over PTY output. + * + * Why shared: terminal-side-effect-authority.md (slice 3) makes main emit + * `pr-link` facts from its per-PTY tracker for local/SSH PTYs, while the + * renderer keeps byte-scanning for remote-runtime PTYs and the kill-switch-off + * path. Both paths must share the carry/dedupe semantics or links split across + * chunks would resolve differently per authority mode. + */ import type { RepoSlug } from './github-links' import { parseGitHubIssueOrPRLink } from './github-links' diff --git a/src/shared/terminal-osc133-command-finished.ts b/src/shared/terminal-osc133-command-finished.ts new file mode 100644 index 00000000000..fc6c9ae7694 --- /dev/null +++ b/src/shared/terminal-osc133-command-finished.ts @@ -0,0 +1,102 @@ +/** + * Chunk-boundary-safe OSC 133;D (command finished) scanner. + * + * Why shared: terminal-side-effect-authority.md (slice 3) makes main emit + * `command-finished` facts from its per-PTY tracker for local/SSH PTYs, while + * the renderer keeps byte-parsing for remote-runtime PTYs and the + * kill-switch-off path. The carry semantics (split prefixes, BEL/ST + * terminators, best-effort exit codes) must be identical in both. + */ + +type OscTerminator = { + index: number + length: number +} + +const OSC_133_PREFIX = '\x1b]133;' +const MAX_OSC_CARRY_LENGTH = 4096 + +function findOscTerminator(data: string, startIndex: number): OscTerminator | null { + const bel = data.indexOf('\x07', startIndex) + const st = data.indexOf('\x1b\\', startIndex) + + if (bel === -1 && st === -1) { + return null + } + if (bel !== -1 && (st === -1 || bel < st)) { + return { index: bel, length: 1 } + } + return { index: st, length: 2 } +} + +function parseBestEffortExitCode(value: string | undefined): number | null { + if (!value) { + return null + } + const parsed = Number.parseInt(value, 10) + return Number.isNaN(parsed) ? null : parsed +} + +function findPrefixCarry(data: string): string { + const maxCarryLength = Math.min(data.length, OSC_133_PREFIX.length - 1) + for (let length = maxCarryLength; length > 0; length -= 1) { + const suffix = data.slice(data.length - length) + if (OSC_133_PREFIX.startsWith(suffix)) { + return suffix + } + } + return '' +} + +export type Osc133CommandFinishedScanner = { + /** Feed one raw PTY chunk; fires once per complete OSC 133;D sequence. */ + scan: (data: string) => void + /** Drop the cross-chunk carry (transport teardown / parser reset). */ + reset: () => void +} + +export function createOsc133CommandFinishedScanner( + onCommandFinished: (bestEffortExitCode: number | null) => void +): Osc133CommandFinishedScanner { + let carry = '' + + const handleOsc133 = (payload: string): void => { + const [sequence, exitCode] = payload.split(';') + if (sequence === 'D') { + onCommandFinished(parseBestEffortExitCode(exitCode)) + } + } + + const scan = (data: string): void => { + let combined = carry + data + carry = '' + + while (combined.length > 0) { + const start = combined.indexOf(OSC_133_PREFIX) + if (start === -1) { + carry = findPrefixCarry(combined) + return + } + + const payloadStart = start + OSC_133_PREFIX.length + const terminator = findOscTerminator(combined, payloadStart) + if (!terminator) { + carry = combined.slice(start) + if (carry.length > MAX_OSC_CARRY_LENGTH) { + carry = carry.slice(carry.length - MAX_OSC_CARRY_LENGTH) + } + return + } + + handleOsc133(combined.slice(payloadStart, terminator.index)) + combined = combined.slice(terminator.index + terminator.length) + } + } + + return { + scan, + reset() { + carry = '' + } + } +} diff --git a/src/shared/terminal-output-side-effects.test.ts b/src/shared/terminal-output-side-effects.test.ts new file mode 100644 index 00000000000..6e7c8b5bf19 --- /dev/null +++ b/src/shared/terminal-output-side-effects.test.ts @@ -0,0 +1,141 @@ +// Why: slice 3 of terminal-side-effect-authority.md adds OSC 133;D +// command-finished and GitHub pr-link scanning to the shared tracker so main +// emits those facts for local/SSH PTYs. These tests pin the chunk-boundary +// carry, exit-code best-effort, dedupe, and synthetic-frame isolation rules. +import { describe, expect, it } from 'vitest' +import { + createTerminalTitleTracker, + type TerminalTitleTrackerCallbacks +} from './terminal-output-side-effects' + +const ESC = '\x1b' +const BEL = '\x07' +const ST = `${ESC}\\` + +type RecordedEvent = + | ['title', string] + | ['bell'] + | ['finished', number | null] + | ['pr', string, number] + +function createRecordingTracker(overrides: TerminalTitleTrackerCallbacks = {}): { + events: RecordedEvent[] + tracker: ReturnType +} { + const events: RecordedEvent[] = [] + const tracker = createTerminalTitleTracker({ + onTitle: (normalized) => events.push(['title', normalized]), + onBell: () => events.push(['bell']), + onCommandFinished: (exitCode) => events.push(['finished', exitCode]), + onPrLink: (link) => events.push(['pr', link.url, link.number]), + ...overrides + }) + return { events, tracker } +} + +describe('createTerminalTitleTracker command-finished facts', () => { + it('emits command-finished with best-effort exit codes', () => { + const { events, tracker } = createRecordingTracker() + + tracker.handleChunk(`before${ESC}]133;A${BEL}prompt${ESC}]133;B${BEL}`) + tracker.handleChunk(`${ESC}]133;C${BEL}running${ESC}]133;D;0${BEL}`) + tracker.handleChunk(`${ESC}]133;D;130${BEL}`) + tracker.handleChunk(`${ESC}]133;D;not-a-number${BEL}`) + tracker.handleChunk(`${ESC}]133;D${BEL}`) + + expect(events).toEqual([ + ['finished', 0], + ['finished', 130], + ['finished', null], + ['finished', null] + ]) + }) + + it('detects OSC 133;D split across chunk boundaries (BEL and ST terminated)', () => { + const { events, tracker } = createRecordingTracker() + + tracker.handleChunk(`chunk${ESC}]133`) + tracker.handleChunk(';D;1') + expect(events).toEqual([]) + tracker.handleChunk(`30${BEL}rest`) + tracker.handleChunk(`${ESC}]133;D;7${ST}`) + + expect(events).toEqual([ + ['finished', 130], + ['finished', 7] + ]) + }) + + it('orders chunk facts titles → command-finished → bell', () => { + const { events, tracker } = createRecordingTracker() + + tracker.handleChunk(`${ESC}]0;zsh${BEL}${ESC}]133;D;0${BEL}done${BEL}`) + + expect(events).toEqual([['title', 'zsh'], ['finished', 0], ['bell']]) + }) +}) + +describe('createTerminalTitleTracker pr-link facts', () => { + it('emits one fact per PR URL including multiple links in one chunk', () => { + const { events, tracker } = createRecordingTracker() + + tracker.handleChunk( + 'see https://github.com/acme/orca/pull/42 and https://github.com/acme/orca/pull/43 \r\n' + ) + + expect(events).toEqual([ + ['pr', 'https://github.com/acme/orca/pull/42', 42], + ['pr', 'https://github.com/acme/orca/pull/43', 43] + ]) + }) + + it('waits for a boundary when a URL splits across chunks and dedupes repeats', () => { + const { events, tracker } = createRecordingTracker() + + tracker.handleChunk('PR: https://github.com/acme/orca/pull/4') + expect(events).toEqual([]) + tracker.handleChunk('2\r\n') + tracker.handleChunk('again https://github.com/acme/orca/pull/42\r\n') + + expect(events).toEqual([['pr', 'https://github.com/acme/orca/pull/42', 42]]) + }) + + it('skips the 133/URL scans entirely when no consumer is registered', () => { + // Mirrors headless serve: no pty:sideEffect consumer means no callbacks, + // so the scanners must not be created (no carry state, no scan cost). + const titles: string[] = [] + const tracker = createTerminalTitleTracker({ + onTitle: (normalized) => titles.push(normalized) + }) + + tracker.handleChunk(`${ESC}]133;D;0${BEL}https://github.com/acme/orca/pull/42\r\n`) + + expect(titles).toEqual([]) + }) +}) + +describe('createTerminalTitleTracker synthetic-frame isolation', () => { + it('never feeds synthetic frames to the 133/PR scanners', () => { + const { events, tracker } = createRecordingTracker() + + tracker.applySyntheticTitleFrame( + `${ESC}]0;⠋ Cursor Agent${BEL}${ESC}]133;D;0${BEL}https://github.com/acme/orca/pull/42\r\n` + ) + + expect(events).toEqual([['title', '⠋ Cursor Agent']]) + }) + + it('keeps a split 133 carry intact across an interleaved synthetic frame', () => { + const { events, tracker } = createRecordingTracker() + + tracker.handleChunk(`out${ESC}]133;D;`) + // An 80ms spinner tick lands between the two halves of the real OSC. + tracker.applySyntheticTitleFrame(`${ESC}]0;⠋ Cursor Agent${BEL}`) + tracker.handleChunk(`130${BEL}`) + + expect(events).toEqual([ + ['title', '⠋ Cursor Agent'], + ['finished', 130] + ]) + }) +}) diff --git a/src/shared/terminal-output-side-effects.ts b/src/shared/terminal-output-side-effects.ts index a1a0ed56a14..5dfe149fcac 100644 --- a/src/shared/terminal-output-side-effects.ts +++ b/src/shared/terminal-output-side-effects.ts @@ -18,6 +18,11 @@ import { normalizeTerminalTitle } from './agent-detection' import { createBellDetector } from './terminal-bell-detector' +import { + createTerminalGitHubPRLinkDetector, + type TerminalGitHubPRLink +} from './terminal-github-pr-link-detector' +import { createOsc133CommandFinishedScanner } from './terminal-osc133-command-finished' /** Ms of title-less output after a working title before it is cleared. */ export const STALE_WORKING_TITLE_TIMEOUT_MS = 3000 @@ -59,6 +64,15 @@ export type TerminalTitleTrackerCallbacks = { * across chunks), after the chunk's title facts — the renderer drain order. */ onBell?: () => void + /** + * Fired per complete OSC 133;D (chunk-boundary-safe) with the sequence's + * best-effort exit code — mirrors the renderer terminal-command-lifecycle + * semantics so the fact path drops stale agent rows exactly like byte mode. + */ + onCommandFinished?: (bestEffortExitCode: number | null) => void + /** Fired once per newly observed GitHub PR URL (chunk-boundary-safe, + * deduplicated per tracker like the renderer detector). */ + onPrLink?: (link: TerminalGitHubPRLink) => void } export type TerminalTitleTracker = { @@ -87,8 +101,22 @@ export function createTerminalTitleTracker( callbacks: TerminalTitleTrackerCallbacks, options: { initialTitle?: string } = {} ): TerminalTitleTracker { - const { onTitle, onAgentBecameIdle, onAgentBecameWorking, onAgentExited, onBell } = callbacks + const { + onTitle, + onAgentBecameIdle, + onAgentBecameWorking, + onAgentExited, + onBell, + onCommandFinished, + onPrLink + } = callbacks const bellDetector = onBell ? createBellDetector() : null + // Why: created only when a consumer exists (like the bell detector) so + // headless serve never pays the per-chunk 133/URL scans. + const commandFinishedScanner = onCommandFinished + ? createOsc133CommandFinishedScanner(onCommandFinished) + : null + const prLinkDetector = onPrLink ? createTerminalGitHubPRLinkDetector() : null // Why: seed both the emitted-title memory (stale-title probe) and the agent // tracker so a mid-session tracker behaves as if it had observed the pane's // last live title — parity with the renderer processor's seeding. @@ -183,6 +211,15 @@ export function createTerminalTitleTracker( } }, STALE_WORKING_TITLE_TIMEOUT_MS) } + // Per-chunk fact order: titles → command-finished → pr-link → bell. The + // bell stays last (the renderer drain's order); the byte scanners keep + // their own cross-chunk carry so split sequences/URLs still resolve. + commandFinishedScanner?.scan(data) + if (prLinkDetector) { + for (const link of prLinkDetector(data)) { + onPrLink?.(link) + } + } if (containsBell) { onBell?.() } @@ -204,6 +241,8 @@ export function createTerminalTitleTracker( // The deliberate permission BEL rides outside the OSC title sequence. A // FRESH detector instance keeps the OSC-terminator-vs-bell semantics // while guaranteeing zero interaction with the chunk detector's state. + // Synthetic frames never reach the 133/PR-link scanners: fabricated bytes + // contain neither and must not perturb their cross-chunk carry state. if (onBell && createBellDetector().chunkContainsBell(frame)) { onBell() } @@ -227,6 +266,7 @@ export function createTerminalTitleTracker( clearStaleTitleTimer() agentTracker?.reset() bellDetector?.reset() + commandFinishedScanner?.reset() } } } diff --git a/src/shared/terminal-side-effect-facts.ts b/src/shared/terminal-side-effect-facts.ts index 961ecf74cfb..b1c3733d071 100644 --- a/src/shared/terminal-side-effect-facts.ts +++ b/src/shared/terminal-side-effect-facts.ts @@ -6,6 +6,8 @@ * See docs/reference/terminal-side-effect-authority.md. */ +import type { TerminalGitHubPRLink } from './terminal-github-pr-link-detector' + /** Why tagged: stale-clear facts come from main's unthrottled 3s timer, not * observed bytes. Renderer policy clears title/cache state from them but * must not schedule task-complete notifications or unread attention — a @@ -16,6 +18,11 @@ export type TerminalSideEffectFact = | { kind: 'agent-working' } | { kind: 'agent-idle'; title: string; staleWorkingTitleClear?: boolean } | { kind: 'agent-exited' } + /** OSC 133;D — foreground shell command exited (exit code best-effort). */ + | { kind: 'command-finished'; exitCode: number | null } + /** Carries the parsed link so the renderer store consumer never re-parses + * the URL (parse drift would break the per-PTY dedupe contract). */ + | { kind: 'pr-link'; link: TerminalGitHubPRLink } export type TerminalSideEffectBatch = { ptyId: string