Repository navigation
agent: restore native TUI with terminal regression harness - #39
Conversation
Assisted-by: Codex
github-actions
Bot
added
documentation
Assisted-by: Codex
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f93ca6ca8f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else { | ||
| ss << " "; | ||
| } | ||
| ss << "\xE2\x86\x91 " << state.stats.total_input << " \xE2\x86\x93 " << state.stats.total_output; |
There was a problem hiding this comment.
The footer emits Unicode arrows here, while the same new TUI also emits a Unicode bullet and prompt chevron. Replace these glyphs and their test expectations with ASCII equivalents as required by the repository's contributor rules.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
| // Scan forward from `pos` (which must point to a '{') and return the index one | ||
| // past the matching closing '}', or std::string::npos if the braces are never | ||
| // balanced. Handles nested objects and arrays, and skips over string literals | ||
| // (including escaped characters) so interior braces inside strings are ignored. |
There was a problem hiding this comment.
This new comment hard-wraps a single explanation across four lines, contrary to the repository requirement that comments are usually one or two lines and are not hard-wrapped. Condense it or remove details already expressed by the implementation.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
| auto cmd_result = run_user_command(cmd, working_dir, g_is_interrupted, | ||
| tui ? [&](std::string_view chunk) { | ||
| tui->post_transcript(std::string(chunk), tui_transcript_style::NORMAL); | ||
| } : std::function<void(std::string_view)>()); |
There was a problem hiding this comment.
When an interactive !cmd is still running, this TUI path never marks the footer as generating. Because raw mode disables terminal signals, Ctrl+C is consumed by the renderer, takes its idle-editor branch, and queues EOF instead of setting g_is_interrupted; a long-running shell command therefore continues, and the agent exits only after it eventually finishes. Mark shell execution as interruptible or route Ctrl+C directly to its interruption flag.
Useful? React with 👍 / 👎.
| case tui_event_type::TRANSCRIPT: | ||
| flush_text_buffer(false); | ||
| append_transcript_line(event.text, event.transcript_style); |
There was a problem hiding this comment.
For TUI !cmd output, each pipe-read chunk is posted as a separate TRANSCRIPT event, and this call treats every event as a complete line. Since the transcript path appends a newline when the chunk lacks one, output exceeding the 4095-byte read buffer or arriving through multiple delayed writes gains spurious line breaks and can corrupt split UTF-8 or ANSI sequences. Buffer these chunks until actual newlines or EOF instead of rendering each callback as a line.
Useful? React with 👍 / 👎.
| std::lock_guard<std::mutex> lock(autocomplete_threads_mu_); | ||
| autocomplete_threads_.push_back(std::move(worker)); |
There was a problem hiding this comment.
While editing any @ completion, every changed query starts another thread, but completed workers remain as joinable entries in autocomplete_threads_ until renderer shutdown. Repeated completion use therefore retains thread resources for the entire session and can also leave many directory scans running concurrently in large directories; join or reuse superseded workers during the session rather than accumulating them.
Useful? React with 👍 / 👎.
Assisted-by: Codex
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8383268f5c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!params.prompt.empty()) { | ||
| initial_prompt = params.prompt; | ||
| params.prompt.clear(); // Only use once | ||
| params.single_turn = true; |
There was a problem hiding this comment.
When stdin is a terminal, -p/--prompt previously supplied the first turn and then left the agent interactive; this assignment now forces every such invocation to exit after that response. The shared CLI exposes -st/--single-turn as the separate opt-in for this behavior, so users can no longer seed an interactive session with -p unless this assignment is removed or restricted to an explicitly non-interactive mode.
Useful? React with 👍 / 👎.
| console::log("\n› "); | ||
| if (tui) { | ||
| tui_command command; | ||
| if (!tui->wait_for_command(command)) { |
There was a problem hiding this comment.
On POSIX, if SIGTERM or an externally delivered SIGINT arrives while the interactive TUI is waiting for input, signal_handler only sets g_is_interrupted, while this indefinite queue wait never polls that flag and no command is enqueued. The process therefore remains running after the first termination signal; the signal path needs to wake this wait or the wait must periodically check the interruption flag.
Useful? React with 👍 / 👎.
| std::string display = truncate_display(event.tool_output, 500); | ||
| if (!display.empty()) { | ||
| if (display.back() != '\n') { | ||
| display.push_back('\n'); | ||
| } | ||
| append_transcript_line(display, event.tool_success ? tui_transcript_style::NORMAL : tui_transcript_style::FAILURE); |
There was a problem hiding this comment.
When a tool fails with an empty output and a populated error - for example, write cannot open its target or receives no file_path - the streaming path at agent-loop.cpp:464 puts only tool_res.output in this event. Consequently this renderer displays only the timing footer and hides the failure reason from the interactive user; carry the error through the event and render it for unsuccessful results.
Useful? React with 👍 / 👎.
| size_t brace_start = visible_content.find('{', body_start); | ||
| if (brace_start == std::string::npos) { | ||
| // No JSON object at all after the opening tag; nothing to do. | ||
| break; | ||
| } | ||
| size_t brace_end = find_balanced_brace_end(visible_content, brace_start); |
There was a problem hiding this comment.
For a complete JSON array whose closing XML tag is missing, such as <tool_calls>[{...},{...}], searching for the first { starts balancing inside the first element rather than at the surrounding [. The fallback therefore parses and executes only the first call, removes that prefix, and leaves the remaining calls as visible assistant text; start at the first JSON object or array delimiter so the entire payload is recovered.
Useful? React with 👍 / 👎.
Assisted-by: Codex
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff12bdf172
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| console::log("\nCompacting...\n"); | ||
| if (command_buffer == "/compact") { | ||
| emit_tui_or_console("\nCompacting...\n", tui_transcript_style::INFO); | ||
| if (agent.compact()) { |
There was a problem hiding this comment.
When /compact performs summary generation, this synchronous call can take as long as a model turn, but the TUI is never marked as generating. handle_input_event only invokes the interruption callback while footer_.generating is true, so ESC does nothing and Ctrl+C queues EOF instead; the compaction continues, and the agent exits only after it finishes. Set the generating state around this call or route cancellation directly to the interruption flag.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| void tui_renderer::render_tool_result(const tui_event & event) { | ||
| std::string display = truncate_display(event.tool_output, 500); |
There was a problem hiding this comment.
For successful TUI tools, this unconditionally truncates the output after the event projection has discarded the rest of tool_result. The read tool stores image bytes and MIME type specifically for terminal preview, while update_plan sets no_truncate_display; the legacy CLI honors both. Consequently the TUI no longer previews images read by vision models and clips long plans, so carry and honor these fields in the TUI event.
Useful? React with 👍 / 👎.
| console::log("\n"); | ||
| } | ||
|
|
||
| // Build user content — multimodal if images were pasted, plain string otherwise |
There was a problem hiding this comment.
This newly added comment contains an em dash despite the repository's explicit ASCII-only requirement; replace it with an ASCII hyphen.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
- read the fixture control pipe with stdio and retry on EINTR: libc++ getline blocked until the pipe buffer filled and SIGWINCH interrupted reads, which is why the macOS PTY tests timed out - install the SIGWINCH handler with SA_RESTART so a resize cannot fail a blocking read on another thread - ignore the transient PENDIN termios bit and resolve the executable path in test_terminal.py; raise the PTY test timeout to 600s - default-initialize permission_request::type (GCC -Werror=uninitialized) - undef the windows.h ERROR macro in http-inference-backend.cpp (MSVC) - buffer shell command output by line in the TUI; mark shell commands and /compact as generating so ESC/Ctrl+C interrupt them; discard a partial compaction summary when interrupted - keep -p interactive (use -st for single turn); exit the idle TUI on SIGTERM - show tool errors and honor no_truncate_display in the TUI - reap finished file completion workers instead of keeping them until exit - recover unterminated <tool_calls> arrays in the tool protocol parser
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48342bfb13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (backend_mode != "local") { | ||
| params.server_base = server_url; | ||
| } |
There was a problem hiding this comment.
When --server-url targets an OpenAI-compatible endpoint without llama-server's /props, this new server_base assignment allows parsing without -m, while the same change removes the only fallback that populated http_cfg.model. The backend consequently sends model: "default", and even a supplied -m is ignored, so generic routers can reject the request or route it to the wrong model. Propagate params.model.get_name() or add an explicit HTTP model option.
Useful? React with 👍 / 👎.
| return lines; | ||
| } | ||
|
|
||
| width = std::max(width, 8); |
There was a problem hiding this comment.
When a slash or file-completion menu is open while the terminal is narrowed below eight columns, this forces each menu row to use an eight-column layout even though current_region_lines() budgets it as one row at term_cols_. The terminal then wraps that row into multiple physical rows, scrolling or overwriting the managed editor/footer and invalidating cursor placement; retain the actual width and use a shorter prefix or truncation marker for very narrow terminals.
Useful? React with 👍 / 👎.
Overview
Reworks the native inline TUI from #37 and adds a repeatable terminal rendering harness. Streaming output, editor input, completion and permission overlays now share the renderer, with fixes for resize, cursor placement, cancellation, terminal cleanup, and image attachment ownership.
The harness drives the production renderer through a PTY, checks interpreted screens and cursors, and replays captured output in xterm.js with screenshots. Adds Linux/macOS/Windows CI and a native Windows console test.
Normal session video
Watch the normal session (9:48, MP4)
The actual agent runs directly in a fixed 100x28 xterm window at normal playback speed, with the real Qwen server. It uses all six built-in tools to fix a small Python project, then demonstrates menus, skills, permissions, shell commands, paste, an actual clipboard image, cancellation, resume, and conversation clearing. Full coverage and observations.
The recording retains rough edges: Alt+Enter does not insert a newline in this xterm setup, and the resumed model picks up the earlier cancelled counting request until interrupted and given a clearer instruction. Multiline clipboard paste works. The clipboard image is correctly read as Tuesday with 25 units;
/compactreports that the conversation is too short to compact. No MCP server is configured for this demo.Terminal test video
Watch the captioned Linux xterm recording (4:06, MP4)
Shows all 36 terminal scenarios with color off/on, a fresh real Qwen session and tool run, and fresh CTest, sanitizer, and browser-replay results. The production renderer's PTY output is displayed live in xterm; fixture events and clipboard bytes are injected. Recording scope and limitations.
The initial fork CI run passed Linux terminal tests, failed the Windows build, and timed out in the macOS PTY tests. The video does not resolve or demonstrate those platform checks.
Menu spacing regression
Opening the command menu previously scrolled the transcript up permanently, leaving six blank rows after dismissal and adding scrollback on repeated openings. Layout height changes now repaint the transcript above the editor without scrolling temporary menu rows. Regression tests check exact transcript rows, dismissal paths, resize, permission completion, repeated cycles, and the live Qwen command menu. An xterm screenshot comparison also confirms identical transcript pixels before opening and after closing. The video above records the earlier commit and predates this fix.
Validation
See
tools/agent/tests/README.mdfor reproduction commands and verification limits. This PR targets this fork only.Requirements