feat(pty): add PTY support for interactive TUI applications by dedene · Pull Request #7 · dedene/claw-wrap

dedene · 2026-02-19T12:39:29Z

Summary

Add PTY mode for wrapped tools so interactive TUI apps (vim, htop, etc.) work correctly with colors and cursor keys
Wrapper requests PTY when terminal attached; daemon honors if tool has use_pty: true
Protocol extended with window size handling and SIGWINCH forwarding

Security

PTY flag included in HMAC signature (prevents downgrade attacks)
Process group isolation preserved via pty.Start()
Output redaction applies to PTY stream
All existing security invariants maintained

Test plan

Unit tests for PTY-aware HMAC functions (auth_test.go)
Config parsing tests for use_pty field (config_test.go)
All existing tests pass
Manual verification with interactive TUI tool (e.g., vim, htop)

Usage

tools:
  vim:
    binary: /usr/bin/vim
    use_pty: true

Enable interactive TUI apps (vim, htop, etc.) to work correctly through claw-wrap with proper colors and cursor key handling. Changes: - Add creack/pty and golang.org/x/term dependencies - Extend protocol with WinSize, UsePTY, and window size messages - Include PTY flag in HMAC signature to prevent downgrade attacks - Add use_pty config option for tools (opt-in per tool) - Implement PTY allocation in daemon executor with SIGWINCH forwarding - Add raw terminal mode and window size tracking in wrapper - Wrapper always requests PTY when terminal attached; daemon decides based on tool config (graceful fallback to pipes) Security: PTY flag signed in HMAC, process group isolation preserved, output redaction applies to PTY stream, audit logging handles PTY output.

Copilot

Pull request overview

This pull request adds PTY (pseudo-terminal) support to enable interactive TUI applications like vim, htop, and other terminal-based tools to work correctly with colors, cursor control, and proper terminal dimensions. The implementation extends the wrapper-daemon protocol to request PTY mode when a terminal is attached, and the daemon honors the request if the tool is configured with use_pty: true.

Changes:

Extended the wire protocol (version 3 → 4) to include PTY request flag, window size, and WINCH signal forwarding
Modified HMAC authentication to include the PTY flag in signatures, preventing downgrade attacks while maintaining backward compatibility
Implemented PTY-aware I/O loops in both wrapper and daemon with raw terminal mode and signal handling

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
internal/protocol/protocol.go	Added WinSize struct, UsePTY and WindowSize fields to ProxyRequest, Rows/Cols to WrapperMessage, and MsgTypeWinSize constant. Bumped protocol version to 4.
internal/wrapper/wrapper.go	Added PTY detection via term.IsTerminal, window size capture, ioLoopPTY with raw terminal mode and SIGWINCH handling, and integrated PTY flag into HMAC computation.
internal/daemon/executor.go	Implemented startProcessWithPTY, ptyOutputPumper, ptyInputPumper, handlePTYWrapperMessage for PTY process lifecycle. Updated finalization, cleanup, and audit logging to handle both PTY and pipe modes.
internal/daemon/daemon.go	Updated HMAC verification to use VerifyHMACWithPTY including the PTY flag.
internal/auth/auth.go	Added ComputeHMACWithPTY and VerifyHMACWithPTY functions that include PTY flag in signature. Maintained backward compatibility by delegating old functions to new ones with usePTY=false.
internal/auth/auth_test.go	Added comprehensive tests for PTY-aware HMAC functions, verifying flag differences and backward compatibility.
internal/config/config.go	Added UsePTY bool field to ToolDef for YAML configuration.
internal/config/config_test.go	Added tests for UsePTY field parsing from YAML and validation.
go.mod	Added github.com/creack/pty v1.1.24 dependency, updated golang.org/x/sys to v0.41.0 and added golang.org/x/term v0.40.0.
go.sum	Added checksums for new dependencies (creack/pty, golang.org/x/term) and updated golang.org/x/sys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T13:02:29Z

internal/wrapper/wrapper.go

+		case <-stdinEOF:
+			msg := &protocol.WrapperMessage{
+				Type: protocol.MsgTypeStdin,
+				EOF:  true,
+			}
+			ndjson.Write(msg)
+			stdinEOF = nil // Disable this case


Setting a channel to nil doesn't disable the select case. Once the channel is closed (line 272), the select case will continuously trigger with the zero value. This creates a busy loop sending EOF messages repeatedly to the daemon. The same issue exists in the non-PTY ioLoop at line 218.

To properly disable a closed channel case in a select, either remove it from the loop entirely or use a separate boolean flag to track whether EOF has been sent.

Copilot · 2026-02-19T13:02:29Z

internal/wrapper/wrapper.go

+		case <-stdinEOF:
+			msg := &protocol.WrapperMessage{
+				Type: protocol.MsgTypeStdin,
+				EOF:  true,
+			}
+			ndjson.Write(msg)


Missing error handling for ndjson.Write call. While the existing ioLoop also ignores errors for stdinEOF messages (line 217), signal messages (line 232), and the PTY winsize messages at line 365, this is inconsistent with the error handling for stdin data chunks (lines 208, 330). For reliability and consistency, all ndjson.Write calls should check for errors, especially when dealing with control messages that affect process behavior.

Copilot · 2026-02-19T13:02:29Z

internal/wrapper/wrapper.go

+				Type:   protocol.MsgTypeSignal,
+				Signal: sigName,
+			}
+			ndjson.Write(msg)


Missing error handling for ndjson.Write call. The signal message write should check for errors like stdin data chunks do (line 330). If the write fails, the daemon may not receive critical signals like SIGINT, leaving processes uninterruptible.

Suggested change

ndjson.Write(msg)

if err := ndjson.Write(msg); err != nil {

return fmt.Errorf("send signal: %w", err)

}

Copilot · 2026-02-19T13:02:29Z

internal/wrapper/wrapper.go

+					Rows: uint16(rows),
+					Cols: uint16(cols),
+				}
+				ndjson.Write(msg)


Missing error handling for ndjson.Write call. Window size changes should check for write errors to ensure the PTY dimensions are properly synchronized with the daemon. Without error handling, if the connection is broken, the wrapper will silently fail to send window size updates which could cause UI rendering issues in interactive applications.

Suggested change

ndjson.Write(msg)

if err := ndjson.Write(msg); err != nil {

return fmt.Errorf("send window size: %w", err)

}

Copilot · 2026-02-19T13:02:30Z

internal/wrapper/wrapper.go

+			if done {
+				exitCode = msg.ExitCode
+				close(doneCh)
+				// Restore terminal before exit
+				term.Restore(stdinFd, oldState)
+				os.Exit(exitCode)


Terminal state is restored twice on the exit path. Line 245 has a deferred term.Restore that will execute when the function returns, but line 317 explicitly restores before os.Exit. The defer will never run when os.Exit is called (since os.Exit terminates immediately), making the defer redundant for the success path. However, if an error occurs and the function returns normally (lines 311, 323), the defer will execute. This works but is confusing.

Consider removing the defer and only calling term.Restore explicitly before each return/exit point, or ensure the defer is the only restoration point by restructuring to return the exit code instead of calling os.Exit directly.

Copilot · 2026-02-19T13:02:30Z

internal/daemon/executor.go

+	e.cmd = exec.CommandContext(e.ctx, e.tool.Binary, e.req.Args...)
+	e.cmd.Dir = e.req.Cwd
+	e.cmd.Env = env
+
+	// Start with PTY - pty.Start handles setting up stdin/stdout/stderr
+	ptmx, err := pty.Start(e.cmd)
+	if err != nil {
+		return fmt.Errorf("pty start: %w", err)
+	}
+	e.ptyMaster = ptmx
+
+	// Get process group ID (process is its own leader in PTY mode)
+	e.pgid = e.cmd.Process.Pid


The comment states "process is its own leader in PTY mode", but the pty.Start function doesn't configure SysProcAttr to create a new process group. The github.com/creack/pty library's Start() function doesn't automatically set Setpgid. This means the process may not be in its own process group, which could affect signal handling and process cleanup.

In non-PTY mode (line 457-459), explicit process group creation is configured via SysProcAttr. The PTY mode should do the same by setting e.cmd.SysProcAttr before calling pty.Start to ensure consistent process group isolation. Without this, killProcessGroup may not behave as expected for PTY processes.

dedene requested a review from Copilot February 19, 2026 12:52

Copilot started reviewing on behalf of dedene February 19, 2026 12:53 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pty): add PTY support for interactive TUI applications#7

feat(pty): add PTY support for interactive TUI applications#7
dedene wants to merge 1 commit intomainfrom
feature/interactive-tui

dedene commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

-			ndjson.Write(msg)
+			if err := ndjson.Write(msg); err != nil {
+				return fmt.Errorf("send signal: %w", err)
+			}

Conversation

dedene commented Feb 19, 2026

Summary

Security

Test plan

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments