Repository navigation
fix(opencode): a terminal pane whose server never came up says so - #1093
Conversation
Closes #881. Agent Code launches the OpenCode TUI with `--port <free>`, where free comes from a loopback probe that RELEASES the port before the TUI binds. When another process takes it in that window the TUI neither paints nor exits: it was still running 25 s later in the recording, with zero PTY bytes. The package already detects this and reports `live-state { connected: false, reason: 'server-unreachable' }` after its connect deadline, and the adapter already forwards it as a transcript-diagnostic. Nothing reads that diagnostic. So the user got a blank pane with no status, no condition and no error — and a composer that would accept text and silently fail, because programmatic delivery goes through the very server that never came up (#877). The adapter now also raises a `jsonl-error`, the route every other unrecoverable channel fault in this file already takes, and withdraws input readiness. The message names the port and says what to do: this pane cannot be pointed at another one, so the remedy is a reload, which allocates a fresh port. The code rides in the message as well as on the Error, because Electron preserves `message` and drops custom Error properties. In the renderer it joins the LIFETIME banner (`transcriptChannelError`) rather than the transient diagnostics: `AgentTerminalLeaf` stands that over the terminal, and this pane is dead until something real changes. Three distinctions are load-bearing, and each has its own test: - A flap is not a death. The adapter checks the REASON, not just `connected === false`, so an SSE drop after a healthy connection — which the package reconnects from — does not stand a permanent "reload this agent" banner over a pane that is about to be fine. - One error per backend GENERATION, so a retry cannot bury the first verdict — and the latch resets on start, because a reload gets a fresh port and a second contested one has to be reported too. - The diagnostic is not latched: recordings want every transition. Tests replay `port-conflict.json` through the real package and the real adapter (a ReplayServer playing the stranger that answered 418 on the contested port), and through the full renderer harness that runs main's SessionManager and forwarder. All fail first: before the fix the three adapter cases time out waiting for an error that never arrives. Mutations, all killed: no jsonl-error 3 failed; renderer drops the lifetime banner 1; no latch 1; latch never resets 1; readiness left ready 1; any disconnect counts as unreachable 1; port omitted 2; reload guidance omitted 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on #1093. 1. `server-unreachable` is a DEADLINE verdict, not a death certificate. The package keeps reconnecting for the life of the instance, so a TUI whose server was merely late — a cold start, a restore herd of panes, a sleep/wake straddling the 30 s connect deadline — connects afterwards and works. The banner this PR added is a LIFETIME banner and nothing ever cleared one, so a healthy pane kept telling the user to reload it, and `managedTranscriptUnavailableReason` kept answering `transcript_unavailable` to every parent agent reading that child over an intact conversation. The reviewer demonstrated it end to end. The recovery signal already existed and already crossed to the renderer — `transcript-diagnostic { connected: true }` — and nothing subscribed to it, which is the same complaint this PR makes about the failure signal. So the feed now carries channel-HEALTH diagnostics alongside content, and the renderer clears this one fault when the live channel connects. Narrow on purpose: only a live-state that says connected, and only over an error this same fault raised. The remote feed gets a documented no-op rather than a subscription: the host does not forward this channel, and pretending otherwise would look wired when it is not. 2. The history-load assertion was vacuous. The scope installs no `loadInitialHistory` unless a test asks for one, so the load threw inside its own catch and the assertion re-read a value nothing had tried to overwrite — the reviewer proved it by reintroducing the whole #864 AC8 regression and watching the case stay green. The history source is now installed and the call asserted, as the sibling AC8 test does. 3. The renderer predicate had no negative test, and `serverUnreachable = true` survived the entire renderer suite — turning every transient diagnostic (`sink_failed`, `final_drain_incomplete`) into a permanent banner, which is precisely the harm `AgentTerminalLeaf` says the field exists to prevent. Covered now, along with a live-state that is still DOWN not clearing anything. 4. `state.connected ||` is redundant against today's package — `server- unreachable` has one emit site with a literal `connected: false` — so no test can distinguish it. Kept, with the equivalence written down rather than left for a reader to wonder about. 5. The renderer harness failed one route where the recording has a stranger owning the whole port. It now uses `ReplayServer.setBlocking`, the helper whose own comment names this fixture. Mutations after these fixes, all killed: never clear on recovery 1 failed; clear on ANY connected diagnostic 3; every jsonl-error a lifetime banner 1; clear on a disconnected live-state 1. Left as a follow-up, not silently: a server that dies AFTER a healthy connection still reports nothing to the user. Every disconnect reason the package can emit implies `everConnected`, so this fix cannot cover it, and it deserves its own decision about what a recoverable mid-session drop should look like. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Finding 1 is right and it is the sharper version of the bug this PR set out to fix: I replaced a silent pane with a pane that lies permanently. Fixed in 1. [HIGH] A recovered pane kept the banner, and parents kept getting I took your "smallest fix" and made the diagnostic readable rather than inventing a second route. The recovery signal already crossed to the renderer and nothing subscribed — which is the complaint this PR makes about the FAILURE signal, one channel over. So The remote feed gets a documented no-op rather than a On PROBE B — routing a live-channel fault through the durable-transcript surface — you are right that this is the underlying shape, and the clearing path is what makes it tolerable rather than correct. The truly correct version is #881's own suggestion (a process-level error that 2. [MEDIUM] The vacuous history assertion. Caught cleanly, and your M18b is the proof I should have run: reintroducing the entire #864 AC8 regression and watching my test stay green. The history source is installed now and the call asserted, exactly as the sibling AC8 test does. The comment says why the install is load-bearing, so the next person does not drop it again. 3. [MEDIUM] No negative test for the predicate. Fixed with a 4. [LOW] 5. [LOW] The harness's stranger. Fixed: 6. [INFERRED] A server that dies mid-session. Your enumeration is now the body of #1097, including why "report that one too" is not the answer (a disconnect is recoverable by design, and reporting every one retrains the user to ignore the banner) and why this PR's recovery path makes a persistent-drop report safe to build on later. Post-fix mutations,
|
Closes #881.
What was wrong
Agent Code launches the OpenCode TUI with
--hostname 127.0.0.1 --port <free>, where "free" comes from a loopback probe that releases the port before the TUI binds. If another process takes it in that window, the TUI neither paints nor exits — in the recording it was still running 25 s later, with zero PTY bytes, and only ended because the recorder killed it.The package already detects this: after its connect deadline it reports
live-state { connected: false, reason: 'server-unreachable' }. The adapter already forwards that as atranscript-diagnostic. Nothing reads it.So the user got a blank pane: no status, no conditions, no error, a process that looks alive — and a composer that would take text and silently fail, because programmatic delivery goes through the very server that never came up (#877).
The fix
The adapter raises a
jsonl-erroras well — the route every other unrecoverable channel fault in this file already takes — and withdraws input readiness. The message names the port and says what to do:The code rides in the message as well as on the Error, because Electron preserves
messageacross IPC and drops custom Error properties — the same reason the adapter's other errors do it.In the renderer it joins the lifetime banner (
transcriptChannelError), not the transient diagnostics.AgentTerminalLeafstands that field over the terminal, and its own comment says why the distinction matters: transient warnings train people to ignore the banner. This pane is dead until something real changes.Three distinctions that are load-bearing
connected === false. An SSE drop after a healthy connection is something 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.Tests
Driven by the recording, not by imagination.
port-conflict.json's own notes are the spec — the contested port answered 418, the TUI never painted, it never exited — so the harness plays a stranger that answers 418 on/eventand writes zero PTY bytes.opencode-terminal-headlesspackage and real sockets: the error, its content, readiness withdrawal, the reload verdict, the repeat guard, and the flap control.SessionManagerand forwarder: the pane shows the error, it is the lifetime banner, Agent Status andmanagedTranscriptUnavailableReasonagree, and a history load does not writereadyover it (the feat(opencode): build opencode-terminal-headless so OpenCode Terminal agents report status and serve MCP reads #864 AC8 failure mode). Plus a control that a healthy pane gets none of this.All fail first: before the fix the three adapter cases time out waiting for an error that never arrives.
Mutations, all killed:
jsonl-error(the bug)npx tsc -bclean;src/providers/opencode+src/renderer/src/workspace/hook/ipc31 files / 225 tests pass.Not done here: the issue's alternative of one automatic respawn on a fresh port. Surfacing is the smaller change and the issue accepts either; an automatic respawn needs its own decision about how a user-visible pane may restart itself, and would sit on top of this signal rather than replace it.
🤖 Generated with Claude Code