Skip to content
Merged
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
7 changes: 7 additions & 0 deletions src/preload/api/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
SessionKind,
SessionJsonlEntriesEvent,
SessionJsonlErrorEvent,
SessionTranscriptDiagnosticEvent,
SessionAgentPtyDataEvent,
SessionScreenEvent,
SessionSemanticEvent,
Expand Down Expand Up @@ -236,6 +237,12 @@ export const sessionApi = {
onSessionJsonlError: (cb: (e: SessionJsonlErrorEvent) => void): Unsub =>
subscribe('session:jsonl-error', cb),

/** Channel HEALTH reports from provider adapters. Main has always sent
* these; until #881 nothing subscribed, so a fault that healed could not
* say so. */
onSessionTranscriptDiagnostic: (cb: (e: SessionTranscriptDiagnosticEvent) => void): Unsub =>
subscribe('session:transcript-diagnostic', cb),

/** Raw PTY bytes for terminal sessions. Claude sessions do NOT
* emit on this channel — they use screen/jsonl-entry instead. */
onSessionTerminalData: (cb: (e: SessionTerminalDataEvent) => void): Unsub =>
Expand Down
1 change: 1 addition & 0 deletions src/preload/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export type {
SessionScreenEvent,
SessionJsonlEntriesEvent,
SessionJsonlErrorEvent,
SessionTranscriptDiagnosticEvent,
SessionConditionsEvent,
SessionProcessStateEvent,
SubAgentToolCall,
Expand Down
182 changes: 182 additions & 0 deletions src/providers/opencode/runtime/opencodeTerminalSession.system.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { createServer } from 'node:http'
import type { AddressInfo } from 'node:net'
import { mkdtempSync, rmSync } from 'node:fs'
import { DatabaseSync } from './opencodeDatabase.testSupport.js'
import { tmpdir } from 'node:os'
Expand Down Expand Up @@ -103,6 +105,69 @@ async function launchAdapter(recording: LiveFixture, seed?: (path: string) => vo
return { session, events, writer, server, pty, script: buildReplayScript(recording) }
}

/**
* The port-conflict recording, launched the way it was recorded.
*
* `launchAdapter` above waits for a live connection before it returns, which is
* why this recording is excluded from the sweep: in it no connection ever
* happens. Its own notes are the spec —
*
* [17.2ms] spawn opencode … --hostname 127.0.0.1 --port 57497
* [25076.7ms] timeout waiting for exit on port conflict
* [25105.6ms] request to the contested port answered 418 (probe blocker)
* [25105.7ms] tui still running after conflict: true
* [25336.4ms] tui exited code=0 signal=15
*
* — and `pty.bytes === 0` with `firstOutputAt === null`. So the contested port
* ANSWERS, with something that is not OpenCode; the TUI neither paints nor
* exits; and the process only ended because the recorder killed it. A server
* that merely refuses connections would be a different, easier failure: the
* one that was actually observed is a live socket owned by a stranger.
*/
async function launchIntoPortConflict(recording: LiveFixture) {
const dbPath = join(dir, `${recording.scenario}-${cleanups.length}.db`)
const writer = new LiveFixtureWriter(dbPath, recording.sessionID, sessionRowFor(recording.sessionID))
cleanups.push(() => writer.close())
// The stranger on the port. 418 is what the recording captured; any non-SSE
// answer does the same thing to the client.
const stranger = createServer((_request, response) => {
response.writeHead(418, { 'content-type': 'text/plain' })
response.end("i'm a teapot")
})
await new Promise<void>(resolve => stranger.listen(0, '127.0.0.1', resolve))
cleanups.push(() => new Promise<void>(resolve => { stranger.closeAllConnections?.(); stranger.close(() => resolve()) }))
const port = (stranger.address() as AddressInfo).port
const url = `http://127.0.0.1:${port}`
// A fresh PTY per spawn: a reload is a second start, and the first PTY is
// dead by then.
const ptys: AdapterPty[] = []
const session = new OpencodeTerminalSession(
{ cwd: '/sandbox/project', resumeSessionId: recording.sessionID },
{
spawnPty: (() => { const next = new AdapterPty(); ptys.push(next); return next }) as never,
prepareLaunch: async (opts): Promise<OpencodeTerminalLaunch> => ({
binary: opts.binary,
args: ['--session', opts.sessionID, '--hostname', '127.0.0.1', '--port', String(port)],
env: { ...opts.env, OPENCODE_SERVER_USERNAME: 'opencode', OPENCODE_SERVER_PASSWORD: PASSWORD },
sessionID: opts.sessionID,
server: { url, username: 'opencode', password: PASSWORD },
dbPath,
}),
// The recorded deadline is 25 s of real waiting. The package's timer is
// what is being exercised, not its duration, so it is shortened rather
// than faked — the adapter must react to the package's real event.
headlessOptions: { heartbeatMs: 0, durablePollIntervalMs: 40, sseInitialBackoffMs: 20, sseMaxBackoffMs: 40, liveConnectDeadlineMs: 250 },
},
)
const events: Event[] = []
for (const name of EVENT_NAMES) session.on(name as never, ((...args: unknown[]) => events.push({ name, args })) as never)
cleanups.push(() => session.stop())
await session.start()
// The recorded PTY produced ZERO bytes, so nothing is written here either.
// That is the whole difficulty: every other signal this pane has is silence.
return { session, events, ptys, port }
}

const semanticOf = (events: Event[]) => events.filter(e => e.name === 'semantic-event').map(e => e.args[0] as { type: string; turnId?: string; phase?: string; fullText?: string })
const processStates = (events: Event[]) => events.filter(e => e.name === 'process-state').map(e => e.args[0] as { active: boolean; status?: string })
const committed = (events: Event[]) =>
Expand Down Expand Up @@ -321,6 +386,123 @@ describe('OpencodeTerminalSession over recorded TUI sessions (real package, real

})

describe('a TUI that never brings up its server says so (#881)', () => {
// Recorded in `port-conflict.json`: Agent Code launches the TUI with a port
// taken from a loopback probe that releases it before the TUI binds, another
// process takes it in that window, and the TUI then neither exits nor paints.
// The package detects this and reports `live-state { connected: false,
// reason: 'server-unreachable' }` after its connect deadline — and nothing in
// the app acted on it. The user got a blank pane: no status, no conditions,
// no error, and a composer that would accept text and silently fail, because
// programmatic delivery goes through the very server that never came up.
it('raises a transcript error the renderer will show, not just a diagnostic nobody reads', async () => {
const recording = loadLiveFixture('port-conflict.json')
const { events, port } = await launchIntoPortConflict(recording)

await waitUntil(
() => events.some(e => e.name === 'jsonl-error' && String((e.args[0] as Error).message).includes('provider_server_unreachable')),
5000,
'unreachable error',
)
const error = events.find(e => e.name === 'jsonl-error')!.args[0] as Error & { code?: string }
// The code travels in the MESSAGE as well as the property: Electron
// preserves `message` across IPC and drops custom Error fields, which is
// why every other adapter error in this file does the same.
expect(error.code).toBe('provider_server_unreachable')
expect(error.message).toContain(String(port))
// A user reading this has to know what to DO. The pane cannot be
// re-pointed at another port, so the remedy is a reload.
expect(error.message.toLowerCase()).toContain('reload')

// The diagnostic is still emitted: it is what recordings and the debug
// bundle read, and this fix adds a user-facing route rather than moving
// the existing one.
expect(events.some(e => e.name === 'transcript-diagnostic'
&& (e.args[0] as { reason?: string }).reason === 'server-unreachable')).toBe(true)
})

it('withdraws input readiness, because prompts go through the server that never came up', async () => {
// The composer's ready hint comes from FIRST PTY OUTPUT, and this TUI
// never paints — so readiness was never granted here and withdrawing it
// looks redundant. It is not: the grace timer is armed by any byte, and a
// TUI that paints its frame and then loses its server (the same
// `server-unreachable` verdict) would otherwise keep a ready composer
// whose every programmatic delivery fails.
const recording = loadLiveFixture('port-conflict.json')
const { events } = await launchIntoPortConflict(recording)

await waitUntil(
() => events.some(e => e.name === 'input-readiness' && (e.args[0] as { ready: boolean }).ready === false
&& (e.args[0] as { reason?: string }).reason === 'provider-not-ready'),
5000,
'readiness withdrawn',
)
})

it('gives a reload its own verdict, because a reload gets a fresh port', async () => {
// The latch is per BACKEND GENERATION, not per pane. The remedy this error
// tells the user about is a reload, and if the new port is contested too
// they have to be told again — a latch that outlived the backend would
// make the second failure silent, which is the bug this issue is about.
const recording = loadLiveFixture('port-conflict.json')
const { session, events } = await launchIntoPortConflict(recording)
await waitUntil(() => events.some(e => e.name === 'jsonl-error'), 5000, 'first verdict')

await session.stop()
const before = events.filter(e => e.name === 'jsonl-error').length
await session.start()

await waitUntil(() => events.filter(e => e.name === 'jsonl-error').length > before, 5000, 'second verdict')
})

it('says it once per generation however many times the package repeats itself', async () => {
// Today the package cannot repeat: `server-unreachable` comes from a
// one-shot deadline timer and `setLiveState` drops an identical state. So
// this drives the ADAPTER's own rule through the real headless emitter
// rather than pretending the package behaves differently — the guard is
// the adapter's, and it should hold whatever the package sends.
const recording = loadLiveFixture('port-conflict.json')
const { session, events } = await launchIntoPortConflict(recording)
await waitUntil(() => events.some(e => e.name === 'jsonl-error'), 5000, 'verdict')
const headless = (session as unknown as { headless: { emit: (name: string, payload: unknown) => void } }).headless

headless.emit('live-state', { connected: false, reason: 'server-unreachable' })
headless.emit('live-state', { connected: false, reason: 'server-unreachable' })

expect(events.filter(e => e.name === 'jsonl-error')).toHaveLength(1)
// The diagnostic is not latched: recordings want every transition.
expect(events.filter(e => e.name === 'transcript-diagnostic'
&& (e.args[0] as { reason?: string }).reason === 'server-unreachable').length).toBeGreaterThan(1)
})

it('does not condemn a pane whose server came up and then flapped', async () => {
// The control, and the reason the adapter checks the REASON rather than
// just `connected === false`. An SSE stream that drops after a healthy
// connection is a transient the package reconnects from; treating it as a
// dead server would stand a permanent "reload this agent" banner over a
// pane that is about to be fine.
const recording = loadLiveFixture('plain.json')
const { session, events, server } = await launchAdapter(recording)

server.dropStreams()
await waitUntil(
() => events.some(e => e.name === 'transcript-diagnostic' && (e.args[0] as { connected?: boolean }).connected === false),
5000,
'the disconnect',
)

expect(events.filter(e => e.name === 'jsonl-error')).toEqual([])
// And it comes back on its own, which is what makes the distinction real.
await waitUntil(
() => events.filter(e => e.name === 'transcript-diagnostic' && (e.args[0] as { connected?: boolean }).connected === true).length > 1,
5000,
'the reconnect',
)
void session
})

})

async function waitUntil(predicate: () => boolean, timeoutMs: number, label: string): Promise<void> {
const deadline = performance.now() + timeoutMs
while (!predicate()) {
Expand Down
54 changes: 51 additions & 3 deletions src/providers/opencode/runtime/opencodeTerminalSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ export class OpencodeTerminalSession extends EventEmitter implements AgentSessio
private pty: IPty | null = null
private headless: OpencodeTerminalHeadless | null = null
private exited = false
/**
* One error per backend generation for an unreachable server (#881).
*
* The package dedupes identical live states, but the adapter must not rely
* on that: a reconnect that fails again is a NEW state transition, and an
* error row per retry would bury the first one under its own repetitions.
* Reset in `start`, because a replacement backend gets a fresh port and
* deserves its own verdict.
*/
private serverUnreachableReported = false
private startGeneration = 0
private importAbort: AbortController | null = null
private ptyDataSubscription: { dispose(): void } | null = null
Expand Down Expand Up @@ -132,6 +142,7 @@ export class OpencodeTerminalSession extends EventEmitter implements AgentSessio
if (this.pty) throw new Error('OpencodeTerminalSession already started')
const generation = ++this.startGeneration
this.exited = false
this.serverUnreachableReported = false
this.emit('input-readiness', { ready: false, reason: 'starting' })

// Start from the complete inherited environment: a GUI-launched app still
Expand Down Expand Up @@ -226,7 +237,7 @@ export class OpencodeTerminalSession extends EventEmitter implements AgentSessio
launch,
})
this.headless = headless
this.forwardHeadless(headless, pty)
this.forwardHeadless(headless, pty, launch.server.url)

// A terminal runtime renders no condition UI of its own. The headless
// publishes an explicit empty snapshot during start(), which clears any
Expand Down Expand Up @@ -257,7 +268,7 @@ export class OpencodeTerminalSession extends EventEmitter implements AgentSessio
* that was busy at the moment of death looking busy in every snapshot taken
* from its last state.
*/
private forwardHeadless(headless: OpencodeTerminalHeadless, pty: IPty): void {
private forwardHeadless(headless: OpencodeTerminalHeadless, pty: IPty, serverUrl: string): void {
headless.on('activity', ({ active, status }) => {
this.emit('process-state', status ? { active, status } : { active })
})
Expand All @@ -282,7 +293,44 @@ export class OpencodeTerminalSession extends EventEmitter implements AgentSessio
// This event reports transport health; activity and conditions still come
// from the live channel and may be stale while disconnected. Durable
// polling can continue, but it cannot prove the TUI's current busy state.
headless.on('live-state', state => this.emit('transcript-diagnostic', { kind: 'opencode-terminal-live-state', ...state }))
headless.on('live-state', state => {
this.emit('transcript-diagnostic', { kind: 'opencode-terminal-live-state', ...state })
// `server-unreachable` is the package's verdict that the TUI's server
// never came up at all — not a reconnect, not a flap. It is reported
// once, after the connect deadline, and only while `everConnected` is
// false.
//
// WHY it needs a second, user-facing route (#881): the diagnostic above
// reaches the renderer and NOTHING reads it. The recorded failure
// (`port-conflict.json`) is a TUI whose port was taken by another
// process between the loopback probe and the bind: it neither paints nor
// exits, so the pane is blank, the process looks alive, and the user is
// given no status, no condition and no error. Every other unrecoverable
// channel fault in this adapter travels as `jsonl-error`, which the
// renderer turns into a transcript error the pane shows.
//
// Readiness goes with it. The composer's ready hint is armed by the
// FIRST PTY BYTE, while programmatic delivery goes through this server
// (#877) — so a TUI that painted and then lost its server would sit
// there looking ready while every delivery failed.
// `state.connected ||` is redundant against today's package and kept
// deliberately: `server-unreachable` is emitted from exactly one site,
// with a literal `connected: false`, while every `connected: true` site
// carries a different reason or none — so no test can distinguish this
// from the reason check alone. It says what the condition MEANS ("the
// channel is down AND it never came up") rather than relying on a
// one-site coincidence in another package.
if (state.connected || state.reason !== 'server-unreachable') return
if (this.serverUnreachableReported) return
this.serverUnreachableReported = true
this.emit('input-readiness', { ready: false, reason: 'provider-not-ready' })
// The code is in the MESSAGE as well as on the Error: Electron preserves
// `message` across IPC and drops custom Error properties, so the message
// is the only place a diagnosis survives the trip to the renderer.
this.emit('jsonl-error', Object.assign(new Error(
`OpenCode's terminal never answered on ${serverUrl}. Another process most likely took that port while the TUI was starting. This pane cannot be pointed at a new port — reload this agent to retry with a fresh one. (provider_server_unreachable)`,
), { code: 'provider_server_unreachable' }))
})
headless.on('exit', ({ exitCode, signal }) => {
// Ignore a stale callback if this wrapper is ever restarted. Production
// creates a new wrapper per backend generation, but identity fencing here
Expand Down
15 changes: 15 additions & 0 deletions src/remote-client/src/WebSocketSessionFeed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
SessionExitEvent,
SessionJsonlEntriesEvent,
SessionJsonlErrorEvent,
SessionTranscriptDiagnosticEvent,
SessionInputReadinessEvent,
SessionProcessStateEvent,
SessionScreenEvent,
Expand Down Expand Up @@ -264,6 +265,20 @@ export class WebSocketSessionFeed implements SessionFeed {
onSessionJsonlError(cb: (e: SessionJsonlErrorEvent) => void): Unsub {
return this.sub('jsonl-error', cb)
}
/**
* Channel-health diagnostics are not relayed to remote clients yet.
*
* WHY a no-op rather than a `sub(...)`: the host does not forward this
* channel over the websocket, so subscribing would wait for frames that
* never arrive and look wired when it is not. The one consumer today (#881,
* clearing an OpenCode terminal's "server never answered" banner when the
* server turns out to be merely late) concerns a pane the phone does not
* render. When the phone does need it, the frame has to be added at the host
* first, and this is where it lands.
*/
onSessionTranscriptDiagnostic(_cb: (e: SessionTranscriptDiagnosticEvent) => void): Unsub {
return () => {}
}
onSessionHistoryBoundary(cb: (e: SessionHistoryBoundaryEvent) => void): Unsub {
return this.sub('history-boundary', cb)
}
Expand Down
Loading
Loading