Skip to content

feat(terminal): instance terminal (xterm.js over WebSocket) prototype#1467

Draft
dawsontoth wants to merge 2 commits into
stagefrom
claude/instance-terminal-prototype
Draft

feat(terminal): instance terminal (xterm.js over WebSocket) prototype#1467
dawsontoth wants to merge 2 commits into
stagefrom
claude/instance-terminal-prototype

Conversation

@dawsontoth

Copy link
Copy Markdown
Contributor

Prototype / draft — for team discussion. Not for merge as-is.

Adds an interactive terminal in Studio (xterm.js over WebSocket), wired to a PTY running inside the customer's Harper instance. First slice of the "Claude in the container" direction; establishes the authenticated WebSocket channel the planned live-log-stream and Node-inspector-bridge features would reuse.

Companion Harper PR: HarperFast/harper#1755

Design (settled with the team)

  • Connects to the operations API port (same origin as getOperationsUrl — one security boundary), via resolveInstanceWsConnection (the WS analog of resolveInstanceConnection).
  • First-message auth: the credential goes in the first WS frame, never in the handshake headers/subprotocol.

What's here

  • src/features/instance/terminal/ — feature module: wire.ts (protocol, mirror of the Harper side), TerminalView.tsx (xterm ↔ socket), resolveInstanceWsConnection.ts, hooks/useSupportsTerminal.ts, index.tsx (page + gates), routes.ts, README.md.
  • Route registered in the instance tree; super_user-gated nav entry (InstanceNavBar.tsx).
  • package.json@xterm/xterm, @xterm/addon-fit.

Gating

  • super_user (nav + page, and again on the Harper side), direct-connection only (mirrors useSupportsLogSSE), and the instance must run Harper with terminal.enabled: true.

Not included

  • A local dev-only preview shim (VITE_TERMINAL_WS_URL) was used to demo the real Studio Terminal page against a mirror PTY server without a from-source Harper. It is intentionally not part of this PR — the committed resolveInstanceWsConnection only derives the URL from the operations endpoint.

Status / caveats

  • Verified end-to-end in the real Studio local-dev UI against a live PTY (super_user login → Terminal tab → live shell).
  • Branch is based on stage@b6e99a2a; stage has since advanced — rebase before merge.

🤖 Generated with Claude Code

…) prototype

Prototype (for team review) of an interactive terminal in Studio, wired to a PTY
running inside the customer's Harper instance container. Pairs with the Harper
component on claude/instance-terminal-prototype. First slice of the "Claude in
the container" direction; establishes the authenticated WebSocket channel the
planned live-log-stream and inspector-bridge features will reuse.

Design (per review): connects to the OPERATIONS API port (same origin as
getOperationsUrl — one security boundary), with FIRST-MESSAGE auth (credential
in the first WS frame, never in the handshake headers/subprotocol).

- src/features/instance/terminal: new feature module (wire.ts, TerminalView.tsx,
  resolveInstanceWsConnection.ts, hooks/useSupportsTerminal.ts, index.tsx,
  routes.ts, README.md).
- Register the route (instance routes.ts) and add a super_user-gated nav entry
  (InstanceNavBar.tsx).
- package.json: @xterm/xterm, @xterm/addon-fit.

Gated three ways: super_user, direct connection only, and the instance must run
Harper with terminal.enabled: true.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a prototype interactive terminal feature using xterm.js over WebSockets, allowing super_users with direct connections to access a shell inside their Harper instance container. The feedback highlights several key improvements for the terminal implementation: explicitly setting the WebSocket binary type to 'arraybuffer' to correctly handle binary frames, preserving the error status in the onclose handler, wrapping the ResizeObserver callback in requestAnimationFrame to avoid loop limit errors, and adding a unique key prop to TerminalView to ensure its internal state resets correctly when switching instances.

if (!auth) {
throw new Error('No operation token or stored credentials for this instance.');
}
ws = new WebSocket(resolved.url, resolved.protocols);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Set ws.binaryType = 'arraybuffer' explicitly to ensure that any binary frames sent by the server are received as ArrayBuffer objects, matching the ev.data instanceof ArrayBuffer check in the onmessage handler. By default, browsers use 'blob' for binary frames, which would cause binary data to be silently ignored.

			ws = new WebSocket(resolved.url, resolved.protocols);
			ws.binaryType = 'arraybuffer';

Comment on lines +94 to +98
ws.onclose = (ev) => {
setStatus('closed');
setDetail(ev.reason ? `${ev.code} ${ev.reason}` : `code ${ev.code}`);
term.write(`\r\n\x1b[90m[connection closed${ev.reason ? `: ${ev.reason}` : ''}]\x1b[0m\r\n`);
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When a connection error occurs, ws.onerror is fired (setting the status to 'error'), immediately followed by ws.onclose. This causes the status to be overwritten to 'closed' ('Disconnected'), hiding the error state from the user. Use a functional state update in setStatus to preserve the 'error' status if it was already set.

Suggested change
ws.onclose = (ev) => {
setStatus('closed');
setDetail(ev.reason ? `${ev.code} ${ev.reason}` : `code ${ev.code}`);
term.write(`\r\n\x1b[90m[connection closed${ev.reason ? `: ${ev.reason}` : ''}]\x1b[0m\r\n`);
};
ws.onclose = (ev) => {
setStatus((prev) => (prev === 'error' ? 'error' : 'closed'));
setDetail(ev.reason ? ev.code + " " + ev.reason : "code " + ev.code);
term.write("\r\n\x1b[90m[connection closed" + (ev.reason ? ": " + ev.reason : "") + "]\x1b[0m\r\n");
};

Comment on lines +107 to +108
const observer = new ResizeObserver(() => safeFit());
observer.observe(container);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Wrap the safeFit() call inside the ResizeObserver callback with requestAnimationFrame to prevent potential "ResizeObserver loop limit exceeded" errors in the browser when the terminal is resized.

		const observer = new ResizeObserver(() => {
			requestAnimationFrame(() => safeFit());
		});
		observer.observe(container);

return (
<div className="flex flex-col gap-4 p-4">
<PrototypeBanner />
<TerminalView entityId={entityId} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Provide a unique key prop (such as entityId) to the TerminalView component. This makes the reset invariant explicit and ensures that all internal state (such as status, detail, and reconnectNonce) is completely reset when switching between different instances.

Suggested change
<TerminalView entityId={entityId} />
<TerminalView key={entityId} entityId={entityId} />
References
  1. When rendering a modal or component that manages internal state for a specific item (such as an edit modal), provide a unique key prop (e.g., the item's ID or name) to make the reset invariant explicit and ensure the internal state is completely reset when switching between items.

Adds @xterm/xterm and @xterm/addon-fit to the lockfile so pnpm's
frozen-lockfile install (CI default) doesn't fail with
ERR_PNPM_OUTDATED_LOCKFILE. Generated with the repo-pinned pnpm@11.9.0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 46.62% 4420 / 9479
🔵 Statements 46.98% 4717 / 10039
🔵 Functions 38.95% 1061 / 2724
🔵 Branches 39.05% 2863 / 7331
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/instance/InstanceNavBar.tsx 5.26% 0% 0% 5.26% 45-175
src/features/instance/routes.ts 100% 100% 100% 100%
src/features/instance/terminal/TerminalView.tsx 0% 0% 0% 0% 19-170
src/features/instance/terminal/index.tsx 0% 0% 0% 0% 12-64
src/features/instance/terminal/resolveInstanceWsConnection.ts 9.09% 0% 0% 9.09% 38-55
src/features/instance/terminal/routes.ts 66.66% 100% 66.66% 66.66% 9
src/features/instance/terminal/wire.ts 57.14% 100% 0% 57.14% 35-43
src/features/instance/terminal/hooks/useSupportsTerminal.ts 0% 100% 0% 0% 12-13
Generated in workflow #1433 for commit a33797e by the Vitest Coverage Report Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant