fix(runtime): preserve terminal list compatibility

This commit is contained in:
Brennan Benson
2026-08-04 01:04:15 -07:00
parent 5ddd57896a
commit 2620878b6f
11 changed files with 60 additions and 34 deletions
+1 -1
View File
@@ -139,7 +139,7 @@ async function chooseWorktree(ws: WebSocket): Promise<string> {
}
async function chooseTerminal(ws: WebSocket, worktree: string): Promise<string> {
const list = await send(ws, 'terminal.list', { worktree })
const list = await send(ws, 'terminal.list', { worktree, includeVisualLayouts: false })
if (!list.ok) {
throw new Error(`terminal.list failed: ${formatResponse(list)}`)
}
+4 -2
View File
@@ -172,7 +172,8 @@ describe('terminal.sendText explicit worktree routing', () => {
expect(delegate.listTerminals).toHaveBeenCalledTimes(1)
expect(delegate.listTerminals).toHaveBeenCalledWith(
'id:worktree-1',
PLUGIN_WORKSPACE_TERMINAL_LIMIT
PLUGIN_WORKSPACE_TERMINAL_LIMIT,
{ includeVisualLayouts: false }
)
expect(delegate.sendTerminal).not.toHaveBeenCalled()
})
@@ -189,7 +190,8 @@ describe('terminal.sendText explicit worktree routing', () => {
expect(delegate.listTerminals).toHaveBeenCalledTimes(1)
expect(delegate.listTerminals).toHaveBeenCalledWith(
'id:worktree-1',
PLUGIN_WORKSPACE_TERMINAL_LIMIT
PLUGIN_WORKSPACE_TERMINAL_LIMIT,
{ includeVisualLayouts: false }
)
expect(delegate.sendTerminal).toHaveBeenCalledTimes(1)
expect(delegate.sendTerminal).toHaveBeenCalledWith(terminalId, {
@@ -14,7 +14,8 @@ export type PluginRuntimeDelegate = {
} | null>
listTerminals(
worktreeSelector?: string,
limit?: number
limit?: number,
opts?: { includeVisualLayouts?: boolean }
): Promise<{ terminals: { handle: string; title: string | null }[] }>
sendTerminal(
handle: string,
@@ -50,7 +51,8 @@ export function bindPluginHostServices(input: {
listWorktreeTerminals: async (worktreeId) => {
const result = await delegate.listTerminals(
`id:${worktreeId}`,
PLUGIN_WORKSPACE_TERMINAL_LIMIT
PLUGIN_WORKSPACE_TERMINAL_LIMIT,
{ includeVisualLayouts: false }
)
return result.terminals
.slice(0, PLUGIN_WORKSPACE_TERMINAL_LIMIT)
+2 -6
View File
@@ -20759,7 +20759,7 @@ describe('OrcaRuntimeService', () => {
).rejects.toThrow('terminal_topology_conflict')
})
it('never claims writable for an orphaned listing entry, and keeps show aligned with the send gate', async () => {
it('keeps orphaned list and show writability aligned with the send gate', async () => {
const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession({
...getDefaultWorkspaceSession(),
tabsByWorktree: { [TEST_WORKTREE_ID]: [] }
@@ -20790,12 +20790,8 @@ describe('OrcaRuntimeService', () => {
const listed = await runtime.listTerminals(`id:${TEST_WORKTREE_ID}`)
const entry = listed.terminals.find((terminal) => terminal.ptyId === 'pty-orphan')
expect(entry).toMatchObject({ orphaned: true, connected: true })
// The listing makes no writability claim it cannot back: `writable` would
// only restate `connected` here.
expect(entry?.writable).toBeUndefined()
expect(entry).toMatchObject({ orphaned: true, connected: true, writable: true })
// ...but the orphan is genuinely writable, so show and send must agree.
const shown = await runtime.showTerminal(entry!.handle)
expect(shown.writable).toBe(true)
await expect(runtime.sendTerminal(entry!.handle, { text: 'hi' })).resolves.toMatchObject({
+4 -8
View File
@@ -15721,7 +15721,9 @@ export class OrcaRuntimeService {
return activeTerminal.terminal
}
}
const listed = await this.listTerminals(worktreeSelector)
const listed = await this.listTerminals(worktreeSelector, undefined, {
includeVisualLayouts: false
})
const first = listed.terminals[0]?.handle
if (first) {
return first
@@ -15968,9 +15970,6 @@ export class OrcaRuntimeService {
return {
...summary,
preview,
// Why: mirrors terminal.send's PTY gate exactly (connected only —
// orphaned handles still accept writes and adoption re-attaches them).
writable: pty.pty.connected,
tabId: pty.pty.tabId ?? pty.record.tabId,
leafId: parsePaneKey(pty.pty.paneKey ?? '')?.leafId ?? pty.record.leafId,
paneRuntimeId: -1,
@@ -15993,7 +15992,6 @@ export class OrcaRuntimeService {
return {
...summary,
preview,
writable: leaf.writable,
paneRuntimeId: leaf.paneRuntimeId,
ptyId: leaf.ptyId,
rendererGraphEpoch: this.rendererGraphEpoch
@@ -30591,9 +30589,7 @@ export class OrcaRuntimeService {
leafId: orphaned ? `pty:${pty.ptyId}` : pane.leafId,
title: getLatestPtyTitle(pty),
connected: pty.connected,
// Why: no `writable` — for a PTY record it would only restate `connected`,
// which is exactly what terminal.send gates on. Callers needing the claim
// use terminal.show.
writable: pty.connected,
lastOutputAt: pty.lastOutputAt,
preview: pty.preview
}
@@ -8,9 +8,10 @@ export type CoordinatorRuntime = {
sendTerminalAgentPrompt(handle: string, prompt: string): Promise<unknown>
listTerminals(
worktreeSelector?: string,
limit?: number
limit?: number,
opts?: { includeVisualLayouts?: boolean }
): Promise<{
terminals: { handle: string; worktreeId: string; connected: boolean; writable?: boolean }[]
terminals: { handle: string; worktreeId: string; connected: boolean; writable: boolean }[]
}>
createTerminal(
worktreeSelector?: string,
@@ -483,7 +484,9 @@ export class Coordinator {
private async getAvailableTerminals(): Promise<string[]> {
try {
const result = await this.runtime.listTerminals(this.opts.worktree)
const result = await this.runtime.listTerminals(this.opts.worktree, undefined, {
includeVisualLayouts: false
})
const dispatched = this.db.listTasks({ status: 'dispatched' })
const busyHandles = new Set<string>()
@@ -501,9 +504,7 @@ export class Coordinator {
t.handle !== this.opts.coordinatorHandle &&
!busyHandles.has(t.handle) &&
t.connected &&
// Why: absent means the runtime makes no claim beyond `connected`
// (record-backed entry); only an explicit `false` disqualifies.
t.writable !== false
t.writable
)
.map((t) => t.handle)
} catch {
@@ -149,7 +149,9 @@ export const ORCHESTRATION_FEDERATION_ATTACH_METHODS: RpcMethod[] = [
created.warning ?? 'Agent-first worktree creation returned no terminal.'
)
}
const listed = await runtime.listTerminals(`id:${created.worktree.id}`)
const listed = await runtime.listTerminals(`id:${created.worktree.id}`, undefined, {
includeVisualLayouts: false
})
appendFederationTerminalEffects(
effects,
listed.terminals,
@@ -170,7 +170,9 @@ export async function createWorkerWorktree(args: {
if (!terminalHandle) {
throw new Error(created.warning ?? 'Agent-first worktree creation returned no terminal.')
}
const listed = await runtime.listTerminals(`id:${created.worktree.id}`)
const listed = await runtime.listTerminals(`id:${created.worktree.id}`, undefined, {
includeVisualLayouts: false
})
const setupTerminalHandle = created.setupReceipt?.terminalHandle
for (const terminal of listed.terminals) {
effects.push({
@@ -634,7 +634,9 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
}
// Why: fan out one message per recipient (independent read-tracking) but share a thread_id for correlation (Section 4.5).
const { terminals } = await runtime.listTerminals()
const { terminals } = await runtime.listTerminals(undefined, undefined, {
includeVisualLayouts: false
})
const handles = resolveGroupAddress(to, from, terminals, (handle: string) =>
runtime.getAgentStatusForHandle(handle)
)
+28
View File
@@ -139,6 +139,34 @@ describe('runRemoteOrcaCli', () => {
return { runtime, db }
}
it.each([
{ argv: ['terminal', 'list'], includeVisualLayouts: true },
{ argv: ['terminal', 'list', '--json'], includeVisualLayouts: false }
])(
'requests terminal layouts according to the legacy SSH output mode',
async ({ argv, includeVisualLayouts }) => {
const runtime = new OrcaRuntimeService()
const listTerminals = vi.spyOn(runtime, 'listTerminals').mockResolvedValue({
terminals: [],
totalCount: 0,
truncated: false
})
const result = await runRemoteOrcaCli(
runtime,
{ argv, cwd: '/home/alice/repo', env: {} },
LEGACY_FALLBACK_OPTIONS
)
expect(result.exitCode).toBe(0)
expect(listTerminals).toHaveBeenCalledWith(undefined, undefined, {
handles: undefined,
requireFreshPtyLiveness: undefined,
includeVisualLayouts
})
}
)
it('uses the remote ORCA_TERMINAL_HANDLE as orchestration sender identity', async () => {
const { runtime, db } = createRuntime()
+1 -6
View File
@@ -448,9 +448,7 @@ export type RuntimeTerminalSummary = {
leafId: string
title: string | null
connected: boolean
// Why: only graph-backed entries can claim writability independently of
// `connected`; record-backed (PTY) entries omit it rather than restate it.
writable?: boolean
writable: boolean
lastOutputAt: number | null
preview: string
}
@@ -577,9 +575,6 @@ export type RuntimeWorktreeTerminalSleepResult = {
)
export type RuntimeTerminalShow = RuntimeTerminalSummary & {
// Why: `terminal show` always states writability — exactly what terminal.send's
// gate enforces, so the two can never disagree.
writable: boolean
paneRuntimeId: number
ptyId: string | null
rendererGraphEpoch: number