Skip to content

feat(cli): add OpenTUI chat interface - #22

Merged
osimuka merged 6 commits into
mainfrom
feat/opentui-chat
Oct 5, 2026
Merged

osimuka merged 6 commits into
mainfrom
feat/opentui-chat

Conversation

@osimuka

@osimuka osimuka commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Add streaming terminal chat with multiline input, cancellation, tool activity, and a plain-chat fallback. Fix renderer mode compatibility and document runtime and FFI requirements.

Summary

Describe the problem and the changes in this pull request.

Validation

  • npm run typecheck
  • npm run build
  • npm test

Checklist

  • Tests added or updated where behavior changed
  • Documentation updated where needed
  • No secrets or generated credentials are included
  • Changes are focused and backward-compatible, or breaking changes are documented

Add streaming terminal chat with multiline input, cancellation, tool activity, and a plain-chat fallback. Fix renderer mode compatibility and document runtime and FFI requirements.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The dependency setup breaks standard Yarn installation on supported Node versions, and FFI flag precedence is evaluated incorrectly.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Adds an OpenTUI-based streaming chat experience while retaining a plain terminal fallback.

Changes:

  • Adds multiline input, streaming output, tool status, and cancellation.
  • Adds runtime selection and OpenTUI compatibility checks.
  • Adds tests, native validation, dependencies, and documentation.
File Description
cli.ts Integrates selectable chat UIs and streaming turns.
api/​core/​library/​cliChatUi.ts Implements UI selection and plain fallback.
api/​core/​library/​cliChatTurn.ts Processes agent streams and cancellation.
api/​core/​library/​cliOpenTui.ts Implements the OpenTUI interface.
tests/​cliChat.test.ts Tests runtime, input, streaming, and cancellation.
scripts/​test-cli-opentui.mjs Adds native renderer validation.
README.md Documents runtime requirements and controls.
package.json Adds scripts and OpenTUI dependencies.
package-lock.json Locks npm dependencies.
yarn.lock Locks Yarn dependencies.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread package.json Outdated
Comment thread api/core/library/cliChatUi.ts Outdated
Make the OpenTUI stack optional and compile non-TUI features when it is absent. Match Node's last-flag-wins ordering across NODE_OPTIONS and CLI arguments, with regression coverage for both review findings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Release builds can silently omit OpenTUI, and SIGHUP handling incorrectly reports successful termination.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity SIGHUP disconnect handler converts hangup into status-0 exit

api/​core/​library/​cliChatUi.ts:137

This SIGHUP listener converts a hangup into a normal status-0 exit because disconnect only closes readline. Supervisors and shell scripts can therefore mistake a dropped terminal for success; set exit code 129 before closing, as the adjacent SIGTERM handler already preserves signal failure semantics.

Medium severity SIGHUP cleanup exits successfully instead of reporting hangup

api/​core/​library/​cliOpenTui.ts:236

Handling SIGHUP with close suppresses Node's default signal termination, so after cleanup this process exits successfully (status 0) instead of reporting the hangup (normally 129). This can cause shells and supervisors to treat a disconnected session as a successful completion; use a dedicated hangup handler that sets the appropriate nonzero exit code before restoring the terminal.

Comment thread scripts/compile.mjs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-platform Yarn resolutions are incomplete, and the native UI test is not part of required CI validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Run native UI test in required CI and release checks

package.json:48

This native UI test is not invoked by test, release, or .github/workflows/ci.yml, so the required PR and release checks can all pass without executing cliOpenTui.ts. Add a Bun/OpenTUI CI job (or otherwise include this script in required validation) so renderer, input, and terminal-lifecycle regressions block merging and publishing.

osimuka and others added 2 commits October 5, 2026 04:50
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

CI setup and OpenTUI tool-result rendering contain unresolved functional defects.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Match tool results by name and ID to prevent cross-tool updates

api/​core/​library/​cliOpenTui.ts:309

When different concurrent tools reuse a runtime toolCallId, this branch ignores the result's tool name and updates whichever row with that ID was inserted first. The stream contract already permits this collision (tests/agent.acpServer.test.ts:1064), and out-of-order completion can therefore display one tool's success/failure on another tool. Include name in the match when it is available.

Medium severity Finish the assistant segment before inserting orphaned tool results

api/​core/​library/​cliOpenTui.ts:318

For an orphan tool-result (a supported stream shape covered in tests/agent.acpServer.test.ts:382), this adds a tool row but leaves the current Markdown renderable active. Any later text delta is appended to that earlier renderable, so it appears before the tool result instead of after it. Finish the current assistant segment before inserting the synthesized result row.

Comment thread .github/workflows/ci.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

CI does not exercise the documented Node.js FFI path or optional-dependency fallback.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity CI omits Node FFI and optional-dependency fallback checks

.github/​workflows/​ci.yml:34

This CI step only exercises the Bun OpenTUI path. The newly advertised Node.js --experimental-ffi path is never mounted in CI, and test:cli-optional is also not run, so regressions in either the Node renderer or the optional-dependency fallback can merge despite the dedicated checks. Please run both scripts here (the Node check can use NODE_OPTIONS=--experimental-ffi so the spawned SIGHUP child inherits FFI).

@osimuka
osimuka merged commit c22215a into main Oct 5, 2026
5 checks passed
@osimuka
osimuka deleted the feat/opentui-chat branch October 5, 2026 04:54
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