Skip to content

Add wave:term component with direct SSE output + /api/terminput input path#2974

Open
Copilot wants to merge 7 commits intomainfrom
copilot/add-new-tsunami-component
Open

Add wave:term component with direct SSE output + /api/terminput input path#2974
Copilot wants to merge 7 commits intomainfrom
copilot/add-new-tsunami-component

Conversation

Copy link
Contributor

Copilot AI commented Mar 4, 2026

This PR introduces a standalone Tsunami terminal element (wave:term) and routes terminal IO outside the normal render/event loop for lower-latency streaming. It adds imperative terminal output (TermWrite) over SSE and terminal input/resize delivery over a dedicated /api/terminput endpoint.

  • Frontend: new wave:term element

    • Added tsunami/frontend/src/element/tsunamiterm.tsx.
    • Uses @xterm/xterm with @xterm/addon-fit.
    • Renders as an outer <div> (style/class/ref target), with xterm auto-fit to that container.
    • Supports ref passthrough on the outer element.
  • Frontend: terminal transport wiring

    • Registered wave:term in tsunami/frontend/src/vdom.tsx.
    • Added SSE listener handling for termwrite in tsunami/frontend/src/model/tsunami-model.tsx, dispatched to the terminal component via a local custom event.
    • onData and onResize now POST directly to /api/terminput as JSON payloads:
      • id
      • data64 (base64 terminal input)
      • termsize (rows, cols) for resize updates
  • Backend: new terminal IO APIs

    • Added /api/terminput handler in tsunami/engine/serverhandlers.go.
    • Added protocol types in tsunami/rpctypes/protocoltypes.go:
      • TermInputPacket, TermWritePacket, TermSize
    • Added engine/client support in tsunami/engine/clientimpl.go:
      • SendTermWrite(id, data64) -> emits SSE event termwrite
      • SetTermInputHandler(...) and HandleTermInput(...)
    • Exposed app-level APIs in tsunami/app/defaultclient.go:
      • TermWrite(id, data64) error
      • SetTermInputHandler(func(TermInputPacket))
  • Example usage

    app.SetTermInputHandler(func(input app.TermInputPacket) {
        // input.Id, input.Data64, input.TermSize.Rows/Cols
    })
    
    _ = app.TermWrite("term1", "SGVsbG8gZnJvbSB0aGUgYmFja2VuZA0K")

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Copilot AI changed the title [WIP] Add new tsunami component with TermWrite functionality Add wave:term component with direct SSE output + /api/terminput input path Mar 4, 2026
Copilot finished work on behalf of sawka March 4, 2026 05:36
@sawka sawka marked this pull request as ready for review March 4, 2026 06:22
}
}

const TsunamiTerm = React.forwardRef<HTMLDivElement, React.HTMLAttributes<HTMLDivElement>>(function TsunamiTerm(
Copy link
Contributor

Choose a reason for hiding this comment

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

CRITICAL: Missing displayName for React.forwardRef component

According to the project's coding guidelines, when using React.memo() or React.forwardRef(), you must add a displayName for the component. This helps with debugging and React DevTools.

Add after the component definition:

TsunamiTerm.displayName = "TsunamiTerm";

@kilo-code-bot
Copy link
Contributor

kilo-code-bot bot commented Mar 4, 2026

Code Review Summary

Status: 4 Issues Found (Previously Reported) | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 1
SUGGESTION 2
Issue Details (click to expand)

All issues were identified in previous review iterations. No new issues found in latest changes.

CRITICAL

File Line Issue
tsunami/frontend/src/element/tsunamiterm.tsx N/A Missing displayName for React.forwardRef component

WARNING

File Line Issue
tsunami/frontend/src/element/tsunamiterm.tsx N/A Potential ref forwarding issue

SUGGESTION

File Line Issue
tsunami/frontend/src/element/tsunamiterm.tsx N/A Inconsistent null check pattern
tsunami/frontend/src/model/model-utils.ts 92 Redundant empty string check
Other Observations (not in diff)

Copyright Year Updates Needed:

  • tsunami/frontend/src/model/model-utils.ts (line 1) - Copyright should be 2026 for modified files
  • tsunami/app/hooks.go (line 1) - Copyright should be 2026 for modified files
  • tsunami/app/defaultclient.go (line 1) - Copyright should be 2026 for modified files
  • tsunami/engine/clientimpl.go (line 1) - Copyright should be 2026 for modified files

Note: Per project rules, files that are modified should use 2026 for the copyright year.

Files Reviewed (11 files)
  • tsunami/app/defaultclient.go - No new issues
  • tsunami/app/hooks.go - No new issues
  • tsunami/cmd/main-tsunami.go - No new issues
  • tsunami/engine/clientimpl.go - No new issues
  • tsunami/frontend/src/element/tsunamiterm.tsx - 3 issues (previously reported)
  • tsunami/frontend/src/model/model-utils.ts - 1 issue (previously reported)
  • tsunami/frontend/src/model/tsunami-model.tsx - No new issues
  • tsunami/frontend/src/types/vdom.d.ts - No new issues
  • tsunami/frontend/src/vdom.tsx - No new issues
  • tsunami/rpctypes/protocoltypes.go - No new issues
  • tsunami/templates/package.json.tmpl - No new issues
  • tsunami/vdom/vdom_types.go - No new issues

Fix these issues in Kilo Cloud

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 5, 2026

Deploying waveterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: b5e358e
Status: ✅  Deploy successful!
Preview URL: https://8f5d475e.waveterm.pages.dev
Branch Preview URL: https://copilot-add-new-tsunami-comp.waveterm.pages.dev

View logs

);

return (
<div {...outerProps} id={id} ref={setOuterRef as React.RefCallback<HTMLDivElement>} onFocus={handleFocus} onBlur={handleBlur}>
Copy link
Contributor

Choose a reason for hiding this comment

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

WARNING: Potential ref forwarding issue

The setOuterRef callback casts to React.RefCallback<HTMLDivElement>, but it expects TsunamiTermElem (which extends HTMLDivElement with custom methods). This cast may cause type safety issues. Consider using React.RefCallback<TsunamiTermElem> or handling the type more explicitly.

terminalRef.current = terminal;

const onDataDisposable = terminal.onData((data) => {
if (id == null || id === "") {
Copy link
Contributor

Choose a reason for hiding this comment

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

SUGGESTION: Inconsistent null check pattern

The code uses id == null || id === "" here and on line 100, but the project guidelines prefer id == null without the explicit empty string check (since == null catches both null and undefined). Consider simplifying to just check if the id is truthy: if (!id) return;

const { op, params } = termOp;
if (op === "termwrite") {
const data64 = params?.[0];
if (typeof data64 === "string" && data64 !== "") {
Copy link
Contributor

Choose a reason for hiding this comment

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

SUGGESTION: Redundant empty string check

The check data64 !== "" is redundant since the parent condition already checks typeof data64 === "string". An empty string is still a valid string type. If you want to skip empty strings, the check is fine, but consider if this is the intended behavior.

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.

2 participants