Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions src/main/agent-hooks/server-opencode-binder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ describe('opencode binder loop', () => {
now: () => Date.now(),
dbPath: () => dbPath,
listPanes: () => [
{ paneKey: PANE_A, directory: DIR, worktreeId: `repo::${DIR}`, shellPid: 111 }
{
paneKey: PANE_A,
directory: DIR,
worktreeId: `repo::${DIR}`,
shellPid: 111,
lastInputAtMs: null,
previousInputAtMs: null
}
],
sweep: async () => [
{
Expand Down Expand Up @@ -246,8 +253,22 @@ describe('OpenCode 1 serve + attach', () => {
now: () => Date.now(),
dbPath: () => dbPath,
listPanes: () => [
{ paneKey: PANE_A, directory: DIR, worktreeId: `repo::${DIR}`, shellPid: 111 },
{ paneKey: PANE_B, directory: DIR, worktreeId: `repo::${DIR}`, shellPid: 211 }
{
paneKey: PANE_A,
directory: DIR,
worktreeId: `repo::${DIR}`,
shellPid: 111,
lastInputAtMs: null,
previousInputAtMs: null
},
{
paneKey: PANE_B,
directory: DIR,
worktreeId: `repo::${DIR}`,
shellPid: 211,
lastInputAtMs: null,
previousInputAtMs: null
}
],
sweep: async () => [
{ pid: 112, ppid: 111, startedAtMs, executable: 'opencode', argv: ['opencode', 'serve'] },
Expand Down
4 changes: 3 additions & 1 deletion src/main/ipc/pty-ipc-mock-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const trackMock: Mock = vi.fn()
export const classifyErrorMock: Mock = vi.fn()
export const registerPtyMock: Mock = vi.fn()
export const unregisterPtyMock: Mock = vi.fn()
export const notePtyInputMock: Mock = vi.fn()
export const setMigrationUnsupportedPtyMock: Mock = vi.fn()
export const clearMigrationUnsupportedPtyMock: Mock = vi.fn()
export const clearMigrationUnsupportedPtysForPaneKeyMock: Mock = vi.fn()
Expand Down Expand Up @@ -167,7 +168,8 @@ export const linuxCliShimModuleMock = () => ({

export const ptyRegistryModuleMock = () => ({
registerPty: registerPtyMock,
unregisterPty: unregisterPtyMock
unregisterPty: unregisterPtyMock,
notePtyInput: notePtyInputMock
})

export const migrationUnsupportedPtyModuleMock = () => ({
Expand Down
82 changes: 82 additions & 0 deletions src/main/ipc/pty/ipc/write-input.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import { listRegisteredPtys, registerPty, unregisterPty } from '../../../memory/pty-registry'
import { ptyOwnership } from '../provider/ownership-state'
import { createPtyWriteInput } from './write-input'

const PTY_ID = 'pty-write-input-stamp'
/** Registered like any other pane, but the mocked provider below serves only PTY_ID. */
const UNBACKED_PTY_ID = 'pty-write-input-unbacked'

const { provider } = vi.hoisted(() => ({ provider: { write: vi.fn() } }))

vi.mock('../provider/registry', () => ({
tryGetProviderForPty: (id: string) => (id === PTY_ID ? provider : undefined)
}))

const mainWindow = {
isDestroyed: () => false,
webContents: { isDestroyed: () => false, send: vi.fn() }
}

function writePtyInput(data: string, ptyId: string = PTY_ID) {
return createPtyWriteInput({
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this fixture is only read through isDestroyed(), webContents.isDestroyed() and webContents.send, all three of which it provides; Electron's BrowserWindow cannot be constructed outside a running app.
mainWindow: mainWindow as never
}).writePtyInput({ id: ptyId, data, inputKind: 'driving' })
}

/** Prompt activity the OpenCode session binder reads; absent until one arrives. */
function promptActivityAt(ptyId: string): number | undefined {
return listRegisteredPtys().find((pty) => pty.ptyId === ptyId)?.lastInputAtMs
}

function register(ptyId: string): void {
registerPty({ ptyId, worktreeId: null, sessionId: null, paneKey: 'tab:leaf', pid: 1 })
}

beforeEach(() => {
ptyOwnership.set(PTY_ID, null)
ptyOwnership.set(UNBACKED_PTY_ID, null)
provider.write.mockReset()
register(PTY_ID)
register(UNBACKED_PTY_ID)
})

afterEach(() => {
ptyOwnership.delete(PTY_ID)
ptyOwnership.delete(UNBACKED_PTY_ID)
unregisterPty(PTY_ID)
unregisterPty(UNBACKED_PTY_ID)
vi.restoreAllMocks()
})

describe('writePtyInput registry stamp', () => {
it('records the keystroke so the session binder can break a same-directory tie', () => {
const before = Date.now()

expect(writePtyInput('hello')).toBe(true)

const stamped = promptActivityAt(PTY_ID)
expect(stamped).toBeTypeOf('number')
expect(stamped).toBeGreaterThanOrEqual(before)
})

it('does not stamp for an automatic terminal query reply', () => {
// xterm answers CPR/DA1 queries on its own; a reply must never age a pane
// into winning a same-directory session tie.
expect(writePtyInput('\x1b[3;1R')).toBe(true)

expect(promptActivityAt(PTY_ID)).toBeUndefined()
expect(provider.write).toHaveBeenCalled()
})

it('leaves the pty unstamped when no provider can serve the write', () => {
// Reaches the provider lookup and gets nothing back, so no bytes are
// dispatched — the pane must not be credited with activity it never had.
expect(writePtyInput('hello', UNBACKED_PTY_ID)).toBe(false)

expect(promptActivityAt(UNBACKED_PTY_ID)).toBeUndefined()
expect(provider.write).not.toHaveBeenCalled()
})
})
19 changes: 19 additions & 0 deletions src/main/ipc/pty/ipc/write-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
isTerminalInputTooLargeWithDeferredMeasurement,
iterateTerminalInputChunks
} from '../../../../shared/terminal-input'
import { notePtyInput } from '../../../memory/pty-registry'
import { isTerminalQueryReply } from '../../../../shared/terminal-query-reply'
import { ptyOwnership } from '../provider/ownership-state'
import { tryGetProviderForPty } from '../provider/registry'
import type { TerminalInputKind } from '../../../../shared/terminal-input-kind'
Expand All @@ -29,6 +31,20 @@ export function isMainWindowPtyIpcEvent(
export type PtyWritePayload = { id: string; data: string; inputKind: TerminalInputKind }
export type PtyViewportClaimPayload = { id: string; cols: number; rows: number }

/**
* Record input against a PTY for the OpenCode session binder. Only called once
* a provider write is actually about to be issued, so an oversized or
* undispatched payload cannot age a pane into winning a same-directory tie.
* Terminal query replies (DA1/DECRQM/CPR …) are written by xterm with nobody
* typing, so they must not count as input either.
*/
function stampPtyInput(id: string, data: string): void {
if (isTerminalQueryReply(data)) {
return
}
notePtyInput(id, Date.now())
}

export function createPtyWriteInput(deps: {
mainWindow?: PtyRendererDelivery
runtime?: OrcaRuntimeService
Expand Down Expand Up @@ -59,6 +75,9 @@ export function createPtyWriteInput(deps: {
id: string,
data: string
): boolean | Promise<boolean> => {
// Reached only after the oversized-input check, so this is the first point
// where bytes are genuinely on their way to the PTY.
stampPtyInput(id, data)
const chunks = iterateTerminalInputChunks(data)
const first = chunks.next()
if (first.done) {
Expand Down
51 changes: 51 additions & 0 deletions src/main/memory/pty-registry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { describe, expect, it } from 'vitest'

import { listRegisteredPtys, notePtyInput, registerPty, unregisterPty } from './pty-registry'

function registration(ptyId: string) {
return { ptyId, worktreeId: null, sessionId: null, paneKey: 'tab:leaf', pid: 1 }
}

describe('notePtyInput', () => {
it('stamps the registration it was asked about', () => {
const ptyId = 'pty-note-input-stamp'
registerPty(registration(ptyId))
try {
notePtyInput(ptyId, 1_700_000_000_000)
expect(find(ptyId)?.lastInputAtMs).toBe(1_700_000_000_000)
} finally {
unregisterPty(ptyId)
}
})

it('ignores a pty the registry never learned', () => {
notePtyInput('pty-never-registered', 1_700_000_000_000)
expect(find('pty-never-registered')).toBeUndefined()
expect(listRegisteredPtys().some((pty) => pty.ptyId === 'pty-never-registered')).toBe(false)
})

it('keeps the stamp it replaces so a submission survives later typing', () => {
const ptyId = 'pty-note-input-history'
registerPty(registration(ptyId))
try {
notePtyInput(ptyId, 1_000)
notePtyInput(ptyId, 2_000)
expect(find(ptyId)?.lastInputAtMs).toBe(2_000)
expect(find(ptyId)?.previousInputAtMs).toBe(1_000)
} finally {
unregisterPty(ptyId)
}
})

it('drops the stamp with the registration', () => {
const ptyId = 'pty-note-input-teardown'
registerPty(registration(ptyId))
notePtyInput(ptyId, 1_700_000_000_000)
unregisterPty(ptyId)
expect(find(ptyId)).toBeUndefined()
})
})

function find(ptyId: string) {
return listRegisteredPtys().find((pty) => pty.ptyId === ptyId)
}
44 changes: 36 additions & 8 deletions src/main/memory/pty-registry.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
/**
* Lightweight side-table that lets the memory collector attribute PTY
* processes back to the worktree that spawned them.
* Lightweight side-table of live local PTYs, keyed by id.
*
* Why a separate module rather than folding it into pty.ts: the PTY
* subsystem has no other reason to know about worktree memory. Keeping
* the registry here means the collector can evolve (e.g. start tracking
* extra per-session metadata) without touching the critical-path spawn
* handler. The write-side is just two calls — `register` on spawn,
* `unregister` on teardown.
* Two consumers share it: the memory collector, which attributes PTY processes
* back to the worktree that spawned them, and (via the session binder) the
* OpenCode session→pane correlation, which reads the prompt-activity stamps the
* PTY write paths leave here. The write side is `register` on spawn,
* `unregister` on teardown, and `notePtyInput` on each prompt-activity write.
*
* Scope: local PTYs only. SSH-backed PTYs execute on a remote host, so
* their memory does not contribute to Orca's process footprint and
Expand All @@ -26,6 +24,22 @@ export type PtyRegistration = {
// storing null lets the collector render a zero-attribution row for that
// PTY instead of throwing and dropping the whole snapshot.
pid: number | null
/**
* ms epoch of the last prompt activity on this PTY — a keystroke from the
* renderer, or a prompt Orca delivered host-side. Absent until one arrives,
* and dropped when the PTY is re-registered under a new incarnation. The
* session binder reads it to decide which of two same-directory panes
* submitted the prompt that created a session (#22838).
*/
lastInputAtMs?: number
/**
* The stamp `lastInputAtMs` replaced. A session row appears *after* the
* prompt that created it, so the creating pane's useful evidence is its
* pre-creation stamp — and if it typed again after submitting, that evidence
* is only visible here. Two slots recover the ordinary "submitted, then kept
* typing" case; a third post-creation write evicts it.
*/
previousInputAtMs?: number
}

const registry = new Map<string, PtyRegistration>()
Expand All @@ -38,6 +52,20 @@ export function unregisterPty(ptyId: string): void {
registry.delete(ptyId)
}

/**
* Record prompt activity against a PTY, keeping the stamp it replaces so a
* pane that submitted a prompt and then kept typing can still show the
* submission. Ids the registry never learned (remote PTYs, already-torn-down
* ones) are ignored so the row cannot be created by input alone.
*/
export function notePtyInput(ptyId: string, nowMs: number): void {
const entry = registry.get(ptyId)
if (entry) {
entry.previousInputAtMs = entry.lastInputAtMs
entry.lastInputAtMs = nowMs
}
}

/** Snapshot of currently-registered local PTYs for the collector to walk. */
export function listRegisteredPtys(): PtyRegistration[] {
return [...registry.values()]
Expand Down
Loading
Loading