design: xterm.js AgentTerminal component spec

[geoffjay]

design: xterm.js AgentTerminal component spec

design: AgentTerminal xterm.js component spec and scaffolding (refs #676)

• ui/design/agent-terminal-spec.md: full visual spec covering all 6 states
  (connecting, connected read-only, connected interactive, reconnecting,
  disconnected, PTY unavailable), tab bar design, xterm.js theme tokens,
  WebSocket protocol, lazy-connect behaviour, and accessibility requirements
• ui/src/hooks/useAgentTerminal.ts: WebSocket + xterm lifecycle hook;
  lazy-connects on first activate(), handles binary PTY frames, resize via
  FitAddon + ResizeObserver (100ms debounce), interactive keyboard forwarding,
  unavailable detection from {"error":"pty_not_supported"} or fast-close
• ui/src/components/agents/AgentTerminal.tsx: terminal component with toolbar
  (StatusBadge + InteractiveToggle), amber interactive-mode banner, and
  TerminalUnavailable fallback card with "View Logs" escape hatch
• ui/src/pages/agents/AgentDetail.tsx: adds Logs | Terminal tab bar above the
  480px panel; both panels stay mounted (hidden attr) to preserve xterm state
  across tab switches; Terminal tab activates lazy WS connect on first show
• ui/src/styles/themes.ts: adds XTERM_THEME export — always-dark palette with
  sunlit-clay-500 cursor accent and WCAG AA verified ANSI 16-colour set
• ui/src/components/agents/index.ts: exports AgentTerminal + AgentTerminalProps

Implementation note: requires bun add @xterm/xterm @xterm/addon-fit before
building. Blocked by #675 (PTY relay endpoint) for full runtime functionality.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com

[geoffjay]

This change is part of the following stack:

• design: xterm.js AgentTerminal component spec #716 ◀

_{Change managed by git-spice.}

[geoffjay]

Review: design: xterm.js AgentTerminal component spec

The spec is thorough — all 6 states covered, a11y requirements clear, lazy-connect + hidden-panel approach is correct. Three issues need fixing before merge.

Blocking

1. Wrong base branch — PR targets main. Per CLAUDE.md all feature work must target feature/autonomous-pipeline. Rebase and update the base before merge.

2. firstDataRef.current never set to true (ui/src/hooks/useAgentTerminal.ts, lines 806–833)

The unavailability heuristic at lines 854–859 checks !firstDataRef.current to detect fast-close-with-no-data. But the flag is never set to true anywhere in the onMessage handler — so any WS disconnect is misclassified as unavailable, including normal reconnects after a brief network blip. Add firstDataRef.current = true when a message is received.

3. binaryType not configured on WebSocketManager (ui/src/hooks/useAgentTerminal.ts, ~line 781)

The message handler checks event.data instanceof ArrayBuffer for binary PTY frames, but the WebSocket default binaryType is 'blob' — binary frames arrive as Blob objects and the check never matches. All PTY output is silently dropped. The spec explicitly requires ws.binaryType = 'arraybuffer'. Either expose a setter on WebSocketManager or add Blob-to-ArrayBuffer fallback handling.

Non-blocking suggestions

4. PTY badge always visible (AgentDetail.tsx, lines 940–942) — spec says the badge should appear only when status is unavailable. Current implementation renders it on every Terminal tab render regardless of connection state.

5. Missing role="tablist" (AgentDetail.tsx, line 910) — the <div> wrapping the tab buttons needs role="tablist" to complete the ARIA tabs pattern.

What's good

• Dynamic import of @xterm packages keeps them out of the initial bundle
• prefers-reduced-motion respected for cursor blink
• Scrollback capped at 5 000 lines — prevents memory growth
• WebSocketManager used directly (not useWebSocket hook) for binary frames — correct per enricher findings
• activatedRef guard prevents double-initialization
• Reconnect separator injected via terminal.write() with ANSI dim styling is clean

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.66%. Comparing base (a5867f0) to head (0b62dc4).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #716   +/-   ##
=======================================
  Coverage   55.66%   55.66%           
=======================================
  Files         126      126           
  Lines       13759    13760    +1     
=======================================
+ Hits         7659     7660    +1     
  Misses       6100     6100           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[geoffjay]

👀 Conductor: Awaiting reviewer — 651 minutes (~10.8 hours) with no review activity.

CI: PASSED — this PR is ready for review. No reviewer has commented or approved yet.

Nudge posted by conductor pipeline sync at 2026-03-23 17:01 UTC. Communicate service offline; using PR comment as fallback.

[geoffjay]

Review: design: AgentTerminal xterm.js component spec and scaffolding

Stack position: design/issue-676 is directly on main but git-spice
reports (needs restack) — the branch is behind main and must be rebased
before merging.

Stack impact note: PR #748 (issue-676) is the full implementation of
the same feature and is open in parallel. Both PRs modify AgentDetail.tsx
and add AgentTerminal.tsx. If either merges first, the other will require
conflict resolution. Since #748 is the canonical implementation, it is worth
coordinating merge order once both are approved.

The spec document is thorough and well-structured. The scaffolding code ships
three correctness bugs that will prevent PTY output from ever reaching the
terminal. All three must be fixed before this lands in main.

---

Blocking — 1. binaryType never set to 'arraybuffer'

File: ui/src/hooks/useAgentTerminal.ts

The hook uses WebSocketManager, which creates new WebSocket(url) inside
_open() without setting binaryType. WebSocket binaryType defaults to
'blob', so binary PTY frames arrive as Blob objects, not ArrayBuffer.

// onMessage handler — this branch is NEVER reached
if (event.data instanceof ArrayBuffer) {
  terminal.write(decoderRef.current.decode(event.data))
  return
}

The instanceof ArrayBuffer check will always be false. PTY output will
fall through to the text-frame handler, which tries to JSON-parse raw binary
and silently drops it. The terminal will display nothing.

WebSocketManager does not expose a binaryType setter. The fix is to
bypass WebSocketManager for this endpoint and use a raw WebSocket with
ws.binaryType = 'arraybuffer' set immediately after construction — the
same approach taken in PR #748 (issue-676).

---

Blocking — 2. firstDataRef is never set to true

File: ui/src/hooks/useAgentTerminal.ts

The 404 / unavailable heuristic reads firstDataRef.current but nothing ever
writes true to it:

// Written to in exactly ONE place — both reset to false
useEffect(() => {
  if (status === 'connecting') {
    connectTimeRef.current = Date.now()
    firstDataRef.current = false     // ← only assignment
  }
  if (status === 'disconnected' && …) {
    if (!firstDataRef.current && elapsed < 800) { // always true
      setStatus('unavailable')
    }
  }
}, [status])

The onMessage handler never does firstDataRef.current = true. Any
connection that closes within 800ms — including valid sessions that send
data and then disconnect quickly — will be misclassified as 'unavailable'.

Fix: set firstDataRef.current = true at the top of the onMessage
callback, before branching on frame type.

---

Blocking — 3. Tab bar container missing role="tablist"

File: ui/src/pages/agents/AgentDetail.tsx

// Container has no role — violates ARIA tablist pattern
<div className="flex border-b border-gray-200 dark:border-gray-700">
  <button role="tab" aria-selected={activeTab === 'logs'} …>Logs</button>
  <button role="tab" aria-selected={activeTab === 'terminal'} …>Terminal</button>
</div>

Per the ARIA spec, role="tab" elements must be owned by a
role="tablist" container. Without it, screen readers will not announce
the group as a tab set. The spec document itself calls for the tablist
pattern.

Fix: add role="tablist" and aria-label="Agent output view" to the
outer <div>.

---

Non-blocking observations

@xterm/xterm version pinned to v5

package.json adds "@xterm/xterm": "^0.5.5". PR #748 uses v6. These are
minor-incompatible releases (addon API changed). Align the version with what
the implementation PR uses to avoid a follow-up bump.

PTY badge always visible

The Terminal tab badge renders unconditionally:

<span className="rounded-full bg-gray-100 px-1.5 py-0.5 …">PTY</span>

The spec says the PTY badge should appear only when the backend reports
unavailability (to serve as a persistent reminder). Consider conditionally
rendering it only when status === 'unavailable', passed from
onStatusChange via the parent's state.

send only accepts string in WebSocketManager

WebSocketManager.send() is typed as send(message: string). Interactive
keyboard input is sent as JSON.stringify({ type: 'input', data }), which
is fine for the JSON text-frame protocol. Be aware that if the backend later
moves to accepting raw binary input frames (as PR #748 does), the manager
would need to accept ArrayBuffer | Uint8Array.

---

Action required before merge

1. Use a raw WebSocket with binaryType = 'arraybuffer' for the PTY
   endpoint — WebSocketManager cannot be used for binary framing.
2. Set firstDataRef.current = true in onMessage on first data receive.
3. Add role="tablist" to the tab bar container.
4. Rebase onto main:

git-spice branch restack
git-spice branch submit