mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
feat(terminal): read the rendered screen with terminal read --screen (STA-4792) (#15380)
* feat(terminal): read the rendered screen with `terminal read --screen` (STA-4792) `terminal read` returns accumulated pty output with escape sequences stripped. That is the right answer for "what happened over time" and the wrong one for "what is on screen": any program that repaints a line comes back as stacked fragments, so one `clear` typed key by key reads as `cclclecleaclear`, and a prompt that draws a space by moving the cursor loses it. Nothing in the output said which question had been answered, so it was used as rendering evidence and produced false conclusions. The runtime already knew how to render — it replays the byte stream through a headless emulator — but only as a fallback for blank reads, alternate screen, and never-attached ptys. A normal attached terminal never reached it. `--screen` asks for it directly. Every read now reports its source, which also surfaces the pre-existing snapshot fallback that until now swapped rendered lines into an ordinary read with no indication. `screen-unavailable` distinguishes "asked for a screen, none could be rendered, here is the stream" from a stream the caller asked for, and an absent source means the host predates the field. Because an older host strips the unknown param and answers with its ordinary read, `--screen` against one fails with that explanation rather than passing the stream off as a screen. `--screen` and `--cursor` are mutually exclusive: a screen is the current frame and has nothing behind it to page. * refactor(terminal): stamp the screen source where rendered lines enter the read Inferring it from tail array identity worked but made a load-bearing contract out of reference equality; any later path spreading the read would silently mislabel. Rendered lines only enter through one builder, so it stamps there and anything still unlabelled is the stream.
This commit is contained in:
@@ -37,6 +37,7 @@ export const BOOLEAN_FLAGS = new Set([
|
||||
'mobile',
|
||||
'mobile-pairing',
|
||||
'no-pairing',
|
||||
'screen',
|
||||
'parent-current',
|
||||
'provision',
|
||||
'ready',
|
||||
|
||||
@@ -76,11 +76,30 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
|
||||
if (cursorFlag !== undefined && cursor === undefined) {
|
||||
throw new RuntimeClientError('invalid_argument', '--cursor must be a non-negative integer')
|
||||
}
|
||||
const screen = flags.get('screen') === true
|
||||
// Why: a cursor pages through accumulated output. A screen read is the current frame and has
|
||||
// nothing behind it to page, so accepting both would imply history that is not there.
|
||||
if (screen && cursorFlag !== undefined) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
'--screen reads the current rendered screen, which has no cursor to page from. Use --cursor without --screen to page through accumulated output.'
|
||||
)
|
||||
}
|
||||
const result = await client.call<{ terminal: RuntimeTerminalRead }>('terminal.read', {
|
||||
terminal: await getTerminalHandle(flags, cwd, client),
|
||||
...(cursor !== undefined ? { cursor } : {}),
|
||||
...(screen ? { screen: true } : {}),
|
||||
limit: getOptionalPositiveIntegerFlag(flags, 'limit')
|
||||
})
|
||||
// Why: an older host drops the unknown `screen` param and answers with its ordinary stream
|
||||
// read, which carries no source. Returning that silently is the exact failure this flag
|
||||
// exists to prevent, so refuse rather than hand back the other question's answer.
|
||||
if (screen && result.result.terminal.source === undefined) {
|
||||
throw new RuntimeClientError(
|
||||
'incompatible_runtime',
|
||||
'This Orca host does not support --screen reads, so it answered with accumulated output instead of the rendered screen. Update Orca on the host, or drop --screen to read accumulated output deliberately.'
|
||||
)
|
||||
}
|
||||
printResult(result, json, formatTerminalRead)
|
||||
},
|
||||
'terminal send': async ({ flags, client, cwd, json }) => {
|
||||
|
||||
@@ -197,17 +197,23 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [
|
||||
{
|
||||
path: ['terminal', 'read'],
|
||||
summary: 'Read bounded terminal output',
|
||||
usage: 'orca terminal read [--terminal <handle>] [--cursor <n>] [--limit <n>] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'terminal', 'cursor', 'limit'],
|
||||
usage:
|
||||
'orca terminal read [--terminal <handle>] [--cursor <n>] [--limit <n>] [--screen] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'terminal', 'cursor', 'limit', 'screen'],
|
||||
notes: [
|
||||
'Omit --terminal to target the active terminal in the current worktree.',
|
||||
'By default this returns accumulated terminal output with escape sequences stripped, not the rendered screen. Any program that repaints a line — shells, progress bars, TUIs — comes back as stacked fragments, so one `clear` keystroke by keystroke reads as `cclclecleaclear`, and spaces a prompt draws by moving the cursor are absent.',
|
||||
'Use --screen to read what the terminal actually renders. Prefer it whenever the answer depends on how output looks rather than what was emitted over time; the default is unsuitable for verifying rendered output.',
|
||||
'The result reports source: stream or screen, so a caller can tell which question was answered. A --screen read falls back to source: stream when no rendered state exists rather than passing the stream off as a screen.',
|
||||
'--screen and --cursor are mutually exclusive: a screen read is the current frame and has no history to page.',
|
||||
'Use --cursor with the nextCursor value from a previous read to get only new output since that read.',
|
||||
'Use --limit to request more retained lines for long agent responses; output reports oldestCursor when older lines were dropped.',
|
||||
'Useful for capturing the response to a command: read before sending, then read --cursor <prev> after waiting.'
|
||||
],
|
||||
examples: [
|
||||
'orca terminal read --json',
|
||||
'orca terminal read --terminal term_abc123 --cursor 42 --limit 1000 --json'
|
||||
'orca terminal read --terminal term_abc123 --cursor 42 --limit 1000 --json',
|
||||
'orca terminal read --terminal term_abc123 --screen --json'
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
@@ -140,11 +140,19 @@ export function formatTerminalRead(result: { terminal: RuntimeTerminalRead }): s
|
||||
const header = [
|
||||
`handle: ${terminal.handle}`,
|
||||
`status: ${terminal.status}`,
|
||||
...(terminal.source ? [`source: ${terminal.source}`] : []),
|
||||
...(terminal.nextCursor !== null ? [`cursor: ${terminal.nextCursor}`] : []),
|
||||
...oldestCursor,
|
||||
...latestCursor,
|
||||
...(terminal.truncated ? ['warning: older output is no longer retained'] : []),
|
||||
...(limitedWarning ? [limitedWarning] : [])
|
||||
...(limitedWarning ? [limitedWarning] : []),
|
||||
// Why: the caller asked for the rendered screen; say plainly that this is not it rather
|
||||
// than let repaint fragments be read as what the terminal displayed.
|
||||
...(terminal.source === 'screen-unavailable'
|
||||
? [
|
||||
'warning: no rendered screen was available, so this is accumulated output; repainted lines may appear as stacked fragments'
|
||||
]
|
||||
: [])
|
||||
]
|
||||
return [...header, '', ...terminal.tail].join('\n')
|
||||
}
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const {
|
||||
callMock,
|
||||
runtimeClientConstructorMock,
|
||||
serveOrcaAppMock,
|
||||
getDefaultUserDataPathMock,
|
||||
addEnvironmentFromPairingCodeMock,
|
||||
listEnvironmentsMock,
|
||||
spawnMock
|
||||
} = vi.hoisted(() => ({
|
||||
callMock: vi.fn(),
|
||||
runtimeClientConstructorMock: vi.fn(),
|
||||
serveOrcaAppMock: vi.fn(),
|
||||
getDefaultUserDataPathMock: vi.fn(() => '/tmp/orca-user-data'),
|
||||
addEnvironmentFromPairingCodeMock: vi.fn(),
|
||||
listEnvironmentsMock: vi.fn(),
|
||||
spawnMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('./runtime-client', async () => {
|
||||
const { createRuntimeClientModuleMock } = await import('./index-test-harness.js')
|
||||
return createRuntimeClientModuleMock({
|
||||
callMock,
|
||||
runtimeClientConstructorMock,
|
||||
serveOrcaAppMock,
|
||||
getDefaultUserDataPathMock
|
||||
})
|
||||
})
|
||||
|
||||
vi.mock('./runtime/environments', () => ({
|
||||
addEnvironmentFromPairingCode: addEnvironmentFromPairingCodeMock,
|
||||
listEnvironments: listEnvironmentsMock,
|
||||
removeEnvironment: vi.fn(),
|
||||
resolveEnvironment: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('child_process', async () => {
|
||||
const { createChildProcessModuleMock } = await import('./index-test-harness.js')
|
||||
return createChildProcessModuleMock(spawnMock)
|
||||
})
|
||||
|
||||
import { main } from './index'
|
||||
import { okFixture, queueFixtures } from './test-fixtures'
|
||||
import { useWorktreeAwarenessEnvironment } from './index-test-harness'
|
||||
|
||||
function readFixture(overrides: Record<string, unknown> = {}) {
|
||||
return okFixture('req_terminal_read', {
|
||||
terminal: {
|
||||
handle: 'term_abc',
|
||||
status: 'running',
|
||||
tail: ['clear'],
|
||||
truncated: false,
|
||||
nextCursor: null,
|
||||
...overrides
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
describe('orca terminal read --screen', () => {
|
||||
useWorktreeAwarenessEnvironment({
|
||||
callMock,
|
||||
serveOrcaAppMock,
|
||||
getDefaultUserDataPathMock,
|
||||
addEnvironmentFromPairingCodeMock,
|
||||
listEnvironmentsMock,
|
||||
spawnMock
|
||||
})
|
||||
|
||||
it('does not ask for a screen unless requested', async () => {
|
||||
queueFixtures(callMock, readFixture({ source: 'stream' }))
|
||||
vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
|
||||
await main(['terminal', 'read', '--terminal', 'term_abc', '--json'], '/tmp/repo')
|
||||
|
||||
expect(callMock).toHaveBeenCalledWith(
|
||||
'terminal.read',
|
||||
expect.not.objectContaining({ screen: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('requests the rendered screen with --screen', async () => {
|
||||
queueFixtures(callMock, readFixture({ source: 'screen' }))
|
||||
vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
|
||||
await main(['terminal', 'read', '--terminal', 'term_abc', '--screen', '--json'], '/tmp/repo')
|
||||
|
||||
expect(callMock).toHaveBeenCalledWith(
|
||||
'terminal.read',
|
||||
expect.objectContaining({ terminal: 'term_abc', screen: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('reports which question was answered in human output', async () => {
|
||||
queueFixtures(callMock, readFixture({ source: 'screen' }))
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
|
||||
await main(['terminal', 'read', '--terminal', 'term_abc', '--screen'], '/tmp/repo')
|
||||
|
||||
expect(String(logSpy.mock.calls[0]?.[0])).toContain('source: screen')
|
||||
})
|
||||
|
||||
// Why: the whole defect is a stream being read as if it were the screen. A fallback has to
|
||||
// announce itself rather than look like a successful screen read.
|
||||
it('warns when a screen was asked for but only the stream was available', async () => {
|
||||
queueFixtures(
|
||||
callMock,
|
||||
readFixture({ source: 'screen-unavailable', tail: ['cclclecleaclear'] })
|
||||
)
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
|
||||
await main(['terminal', 'read', '--terminal', 'term_abc', '--screen'], '/tmp/repo')
|
||||
|
||||
const printed = String(logSpy.mock.calls[0]?.[0])
|
||||
expect(printed).toContain('no rendered screen was available')
|
||||
expect(printed).toContain('stacked fragments')
|
||||
})
|
||||
|
||||
// Why: zod strips the unknown `screen` key on an older host, so it answers with an ordinary
|
||||
// stream read. Handing that back would be the original defect wearing the new flag's name.
|
||||
it("refuses to pass an older host's stream off as a screen read", async () => {
|
||||
queueFixtures(callMock, readFixture({ tail: ['cclclecleaclear'] }))
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const priorExitCode = process.exitCode
|
||||
|
||||
await main(['terminal', 'read', '--terminal', 'term_abc', '--screen', '--json'], '/tmp/repo')
|
||||
|
||||
expect([...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n')).toContain(
|
||||
'does not support --screen reads'
|
||||
)
|
||||
expect(process.exitCode).toBe(1)
|
||||
|
||||
process.exitCode = priorExitCode
|
||||
})
|
||||
|
||||
it('rejects --screen with --cursor instead of implying history that is not there', async () => {
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const priorExitCode = process.exitCode
|
||||
|
||||
await main(
|
||||
['terminal', 'read', '--terminal', 'term_abc', '--screen', '--cursor', '42', '--json'],
|
||||
'/tmp/repo'
|
||||
)
|
||||
|
||||
expect(callMock).not.toHaveBeenCalled()
|
||||
expect([...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n')).toContain(
|
||||
'has no cursor to page from'
|
||||
)
|
||||
expect(process.exitCode).toBe(1)
|
||||
|
||||
process.exitCode = priorExitCode
|
||||
})
|
||||
})
|
||||
@@ -18332,14 +18332,16 @@ export class OrcaRuntimeService {
|
||||
|
||||
async readTerminal(
|
||||
handle: string,
|
||||
opts: { cursor?: number; limit?: number } = {}
|
||||
opts: { cursor?: number; limit?: number; screen?: boolean } = {}
|
||||
): Promise<RuntimeTerminalRead> {
|
||||
const pty = this.getLivePtyForHandle(handle)
|
||||
if (pty) {
|
||||
const read = this.readPtyTerminal(handle, pty.pty, opts)
|
||||
const visibleRead = await this.withVisibleSnapshotFallback(pty.pty.ptyId, read, opts)
|
||||
const visibleRead = opts.screen
|
||||
? await this.readRenderedScreen(pty.pty.ptyId, read, opts)
|
||||
: await this.withVisibleSnapshotFallback(pty.pty.ptyId, read, opts)
|
||||
this.assertLiveTerminalHandleTargetsPty(handle, pty.pty.ptyId)
|
||||
return visibleRead
|
||||
return labelTerminalReadSource(visibleRead)
|
||||
}
|
||||
|
||||
const { leaf } = this.getLiveLeafForHandle(handle)
|
||||
@@ -18355,11 +18357,33 @@ export class OrcaRuntimeService {
|
||||
limit: opts.limit
|
||||
})
|
||||
if (!leaf.ptyId) {
|
||||
return read
|
||||
return { ...read, source: opts.screen ? 'screen-unavailable' : 'stream' }
|
||||
}
|
||||
const visibleRead = await this.withVisibleSnapshotFallback(leaf.ptyId, read, opts)
|
||||
const visibleRead = opts.screen
|
||||
? await this.readRenderedScreen(leaf.ptyId, read, opts)
|
||||
: await this.withVisibleSnapshotFallback(leaf.ptyId, read, opts)
|
||||
this.assertLiveTerminalHandleTargetsPty(handle, leaf.ptyId)
|
||||
return visibleRead
|
||||
return labelTerminalReadSource(visibleRead)
|
||||
}
|
||||
|
||||
// Why: the default read is the accumulated pty stream, which stacks every repaint of a line
|
||||
// ("cclclecleaclear" for one `clear`) and drops spaces a prompt draws with cursor-forward.
|
||||
// That is the right answer for "what happened over time" and the wrong one for "what is on
|
||||
// screen", so an explicit screen read goes to the emulator state instead. When no rendered
|
||||
// state exists the stream is still returned, but labelled `stream` rather than passed off as
|
||||
// a screen — silently answering the other question is the defect this exists to stop.
|
||||
private async readRenderedScreen(
|
||||
ptyId: string,
|
||||
read: RuntimeTerminalRead,
|
||||
opts: { limit?: number } = {}
|
||||
): Promise<RuntimeTerminalRead> {
|
||||
const visibleState = await this.readVisibleTerminalState(ptyId)
|
||||
const lines =
|
||||
visibleState?.lines ?? (await this.readProviderTerminalTailLines(ptyId, opts.limit))
|
||||
if (lines.length === 0) {
|
||||
return { ...read, source: 'screen-unavailable' }
|
||||
}
|
||||
return buildVisibleSnapshotReadFallback(read, lines, opts.limit)
|
||||
}
|
||||
|
||||
// Why a cache: leaf-branch sends may arrive per keystroke; one proven-absent
|
||||
@@ -39560,6 +39584,15 @@ function visibleNonBlankTerminalLines(lines: string[]): string[] {
|
||||
return lines.map((line) => line.trimEnd()).filter((line) => line.trim().length > 0)
|
||||
}
|
||||
|
||||
// Why: every read carries its source, so a caller that asked for a screen and got a response
|
||||
// with no source at all knows it reached a host that predates screen reads — rather than
|
||||
// mistaking the stream for the screen. Rendered lines only ever enter a read through
|
||||
// buildVisibleSnapshotReadFallback, which stamps `screen` itself, so anything still unlabelled
|
||||
// here is the accumulated stream.
|
||||
function labelTerminalReadSource(resolved: RuntimeTerminalRead): RuntimeTerminalRead {
|
||||
return resolved.source ? resolved : { ...resolved, source: 'stream' }
|
||||
}
|
||||
|
||||
function buildVisibleSnapshotReadFallback(
|
||||
read: RuntimeTerminalRead,
|
||||
visibleLines: string[],
|
||||
@@ -39576,7 +39609,8 @@ function buildVisibleSnapshotReadFallback(
|
||||
tail: charBoundedTail.tail,
|
||||
limited:
|
||||
read.limited || lineBoundedTail.length < visibleLines.length || charBoundedTail.limited,
|
||||
returnedLineCount: charBoundedTail.tail.length
|
||||
returnedLineCount: charBoundedTail.tail.length,
|
||||
source: 'screen'
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -882,7 +882,10 @@ const TerminalRead = TerminalHandle.extend({
|
||||
})
|
||||
)
|
||||
.optional(),
|
||||
limit: OptionalFiniteNumber
|
||||
limit: OptionalFiniteNumber,
|
||||
// Why: optional so an older host that does not understand it simply drops the key and answers
|
||||
// with its usual stream read; the response's `source` is what tells the caller which it got.
|
||||
screen: z.literal(true).optional()
|
||||
})
|
||||
|
||||
// Why: preserve the legacy contract — `title: string | null` only, `undefined` rejected, so the CLI's "reset" signal stays distinct.
|
||||
@@ -1184,7 +1187,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [
|
||||
handler: async (params, { runtime }) => ({
|
||||
terminal: await runtime.readTerminal(params.terminal, {
|
||||
cursor: params.cursor,
|
||||
limit: params.limit
|
||||
limit: params.limit,
|
||||
screen: params.screen
|
||||
})
|
||||
})
|
||||
}),
|
||||
|
||||
@@ -646,6 +646,13 @@ export type RuntimeTerminalRead = {
|
||||
nextCursor: string | null
|
||||
latestCursor?: string
|
||||
returnedLineCount?: number
|
||||
// Why: these are two different questions and they disagree whenever a program repaints.
|
||||
// `stream` is the accumulated pty output with escapes stripped, so a redrawn line arrives as
|
||||
// stacked fragments; `screen` is what the terminal actually renders. Naming the source keeps
|
||||
// a caller that asked for one and got the other from reading the answer as the wrong thing.
|
||||
// `screen-unavailable` means a screen was asked for, none could be rendered, and this is the
|
||||
// stream instead — distinct from `stream`, which is the caller getting what they asked for.
|
||||
source?: 'stream' | 'screen' | 'screen-unavailable'
|
||||
}
|
||||
|
||||
export type RuntimeTerminalRename = {
|
||||
|
||||
Reference in New Issue
Block a user