Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions internal/execution/process_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,49 @@ func (manager *ProcessManager) Continue(ctx context.Context, input ProcessContin
return result, nil
}

// WriteInput writes bytes to a retained interactive process's stdin without
// collecting output. Unlike Continue it never drains the pending output
// buffer, so a concurrent write_stdin poll still sees everything the process
// emitted; callers that only need the rolling tail use Snapshot instead.
func (manager *ProcessManager) WriteInput(id int, data []byte) error {
process, ok := manager.get(id)
if !ok {
return ErrProcessNotFound
}
process.touch()
if len(data) == 0 {
return nil
}
if !process.tty || process.stdin == nil {
return ErrProcessStdinDisabled
}
if _, err := process.stdin.Write(data); err != nil && !process.doneClosed() {
return err
}
return nil
}

// ResizeInput updates the PTY window size of a retained interactive process
// so an attached terminal can fill its viewport. Non-positive dimensions are
// a no-op; platforms without PTY support report the transport's error.
func (manager *ProcessManager) ResizeInput(id int, cols, rows int) error {
if cols <= 0 || rows <= 0 {
return nil
}
process, ok := manager.get(id)
if !ok {
return ErrProcessNotFound
}
process.touch()
if !process.tty || process.stdin == nil {
return ErrProcessStdinDisabled
}
if err := resizePTY(process.stdin, cols, rows); err != nil && !process.doneClosed() {
return err
}
return nil
}

func clampInitialProcessWait(wait time.Duration) time.Duration {
return min(wait, maxInteractiveYield)
}
Expand Down
121 changes: 121 additions & 0 deletions internal/execution/process_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package execution

import (
"context"
"errors"
"io"
"os"
"os/exec"
Expand Down Expand Up @@ -85,6 +86,126 @@ func TestProcessManagerInterruptsRetainedProcess(t *testing.T) {
}
}

func TestProcessManagerWriteInputUnknownProcess(t *testing.T) {
manager := NewProcessManager(ProcessManagerOptions{})
if err := manager.WriteInput(4242, []byte("x")); !errors.Is(err, ErrProcessNotFound) {
t.Fatalf("WriteInput unknown id = %v, want ErrProcessNotFound", err)
}
}

func TestProcessManagerWriteInputRejectsPipeProcess(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("test command uses a POSIX shell")
}
root := t.TempDir()
manager := NewProcessManager(ProcessManagerOptions{})
command := exec.Command("/bin/sh", "-c", "sleep 30")
started, err := manager.Start(context.Background(), ProcessStart{
Prepared: PreparedCommand{Command: command}, Request: processManagerRequest(root, command),
}, time.Millisecond)
if err != nil {
t.Fatalf("Start: %v", err)
}
defer manager.Stop(started.ProcessID)
if started.TTY {
t.Fatal("pipe process reported a TTY")
}
if err := manager.WriteInput(started.ProcessID, []byte("x")); !errors.Is(err, ErrProcessStdinDisabled) {
t.Fatalf("WriteInput pipe process = %v, want ErrProcessStdinDisabled", err)
}
if err := manager.WriteInput(started.ProcessID, nil); err != nil {
t.Fatalf("WriteInput empty data = %v, want nil", err)
}
}

func TestProcessManagerWriteInputDoesNotDrainPendingOutput(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("PTY sessions are only available on Linux")
}
root := t.TempDir()
manager := NewProcessManager(ProcessManagerOptions{})
command := exec.CommandContext(context.Background(), "cat")
started, err := manager.Start(context.Background(), ProcessStart{
Prepared: PreparedCommand{Command: command}, Request: processManagerRequest(root, command),
CommandText: "cat", TTY: true,
}, 50*time.Millisecond)
if err != nil {
t.Skipf("PTY transport unavailable: %v", err)
}
if !started.TTY {
t.Skip("PTY transport fell back to pipes")
}
defer manager.Stop(started.ProcessID)

if err := manager.WriteInput(started.ProcessID, []byte("hello\r")); err != nil {
t.Fatalf("WriteInput: %v", err)
}
// The PTY echoes input back into the output stream, so the rolling recent
// tail shows what was typed without anything being drained.
deadline := time.Now().Add(5 * time.Second)
for {
snapshot, ok := manager.Snapshot(started.ProcessID)
if ok && strings.Contains(snapshot.RecentOutput, "hello") {
break
}
if time.Now().After(deadline) {
t.Fatalf("recent output never echoed the input: %q", snapshot.RecentOutput)
}
time.Sleep(10 * time.Millisecond)
}
// WriteInput must not have consumed the pending buffer: a write_stdin-style
// Continue still collects the echoed bytes.
continued, err := manager.Continue(context.Background(), ProcessContinue{
ProcessID: started.ProcessID, Wait: 200 * time.Millisecond,
})
if err != nil {
t.Fatalf("Continue: %v", err)
}
if !strings.Contains(continued.Output, "hello") {
t.Fatalf("WriteInput drained the pending output: continued output = %q", continued.Output)
}
}

func TestProcessManagerResizeInputUnknownProcess(t *testing.T) {
manager := NewProcessManager(ProcessManagerOptions{})
if err := manager.ResizeInput(4242, 100, 40); !errors.Is(err, ErrProcessNotFound) {
t.Fatalf("ResizeInput unknown id = %v, want ErrProcessNotFound", err)
}
}

func TestProcessManagerResizeInputUpdatesWindowSize(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("PTY sessions are only available on Linux")
}
root := t.TempDir()
manager := NewProcessManager(ProcessManagerOptions{})
command := exec.CommandContext(context.Background(), "/bin/sh", "-c", "sleep 0.3; stty size")
started, err := manager.Start(context.Background(), ProcessStart{
Prepared: PreparedCommand{Command: command}, Request: processManagerRequest(root, command),
CommandText: "stty size", TTY: true,
}, time.Millisecond)
if err != nil {
t.Skipf("PTY transport unavailable: %v", err)
}
if !started.TTY {
t.Skip("PTY transport fell back to pipes")
}
defer manager.Stop(started.ProcessID)

if err := manager.ResizeInput(started.ProcessID, 100, 40); err != nil {
t.Fatalf("ResizeInput: %v", err)
}
continued, err := manager.Continue(context.Background(), ProcessContinue{
ProcessID: started.ProcessID, Wait: 2 * time.Second,
})
if err != nil {
t.Fatalf("Continue: %v", err)
}
if combined := started.Output + continued.Output; !strings.Contains(combined, "40 100") {
t.Fatalf("stty size output = %q, want %q", combined, "40 100")
}
}

func TestManagedProcessTerminateSkipsReapedProcess(t *testing.T) {
reaped := make(chan struct{})
close(reaped)
Expand Down
4 changes: 4 additions & 0 deletions internal/execution/pty_fallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ import (
func startPTYProcess(_ *exec.Cmd, _ io.Writer) (io.WriteCloser, func(), error) {
return nil, nil, errors.New("pty transport is unavailable on this platform")
}

func resizePTY(_ io.Writer, _, _ int) error {
return errors.New("pty transport is unavailable on this platform")
}
16 changes: 16 additions & 0 deletions internal/execution/pty_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func openPTY() (*os.File, *os.File, error) {
_ = master.Close()
return nil, nil, err
}
// A fresh PTY defaults to a 0x0 window; seed a sane size so the process
// never sees zero dimensions before an attached client reports its own.
_ = unix.IoctlSetWinsize(masterFD, unix.TIOCSWINSZ, &unix.Winsize{Row: 24, Col: 80})
pts, err := unix.IoctlGetInt(masterFD, unix.TIOCGPTN)
if err != nil {
_ = master.Close()
Expand Down Expand Up @@ -80,3 +83,16 @@ func hardenPTYProcess(command *exec.Cmd) {
return nil
}
}

// resizePTY reports a new window size to the PTY behind an interactive
// process's stdin writer, so full-screen attached terminals drive the
// session's own line wrapping.
func resizePTY(w io.Writer, cols, rows int) error {
file, ok := w.(interface{ Fd() uintptr })
if !ok {
return errors.New("pty transport is unavailable on this platform")
}
return unix.IoctlSetWinsize(int(file.Fd()), unix.TIOCSWINSZ, &unix.Winsize{
Row: uint16(rows), Col: uint16(cols),
})
}
37 changes: 32 additions & 5 deletions internal/tools/exec_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ type ExecSessionSnapshot = execution.ProcessSnapshot

type ExecSessionController interface {
ExecSessions() []ExecSessionSnapshot
ExecSession(id int) (ExecSessionSnapshot, bool)
StopExecSession(id int) bool
StopAllExecSessions() []int
WriteExecSessionInput(id int, data []byte) error
ResizeExecSession(id int, cols, rows int) error
}

type execCommandTool struct {
Expand Down Expand Up @@ -95,7 +98,7 @@ func NewScopedExecCommandTool(workspaceRoot string, scope PathScope, manager *ex
},
"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},
"prefix_rule": {Type: "array", Items: &PropertySchema{Type: "string"}, Description: "Reusable approval prefix for this command, only with `sandbox_permissions: \"require_escalated\"`; keep it narrow, for example [\"git\", \"pull\"]."},
"tty": {Type: "boolean", Description: "True allocates a PTY for the command; false or omitted uses plain pipes.", Default: false},
"tty": {Type: "boolean", Description: "True allocates a PTY for the command; false or omitted uses plain pipes. Use true for commands that may prompt for input (sudo, ssh, interactive installers): the user can then type into the session directly. Setuid tools such as sudo also need sandbox_permissions \"require_escalated\", since the sandbox blocks privilege escalation.", Default: false},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
Required: []string{"cmd"},
AdditionalProperties: false,
Expand Down Expand Up @@ -131,6 +134,10 @@ func (tool execCommandTool) ExecSessions() []ExecSessionSnapshot {
return tool.manager.List()
}

func (tool execCommandTool) ExecSession(id int) (ExecSessionSnapshot, bool) {
return tool.manager.Snapshot(id)
}

func (tool execCommandTool) StopExecSession(id int) bool {
return tool.manager.Stop(id)
}
Expand All @@ -139,6 +146,14 @@ func (tool execCommandTool) StopAllExecSessions() []int {
return tool.manager.StopAll()
}

func (tool execCommandTool) WriteExecSessionInput(id int, data []byte) error {
return tool.manager.WriteInput(id, data)
}

func (tool execCommandTool) ResizeExecSession(id int, cols, rows int) error {
return tool.manager.ResizeInput(id, cols, rows)
}

func (tool execCommandTool) run(ctx context.Context, args map[string]any, engine *zeroSandbox.Engine, directBudget bool) Result {
commandText, err := execCommandArg(args)
if err != nil {
Expand Down Expand Up @@ -172,8 +187,14 @@ func (tool execCommandTool) run(ctx context.Context, args map[string]any, engine
if issue := detectShellCommandIssueForRuntime(commandText, detectShellRuntime(runtimeGOOS())); issue != nil && !msysGuardBypassed(issue, commandEngine) {
return shellIssueBlockResult(*issue)
}
if interactive := zeroSandbox.DetectInteractiveCommand(commandText, runtimeGOOS()); interactive.Interactive {
return interactiveBlockResult(interactive)
// tty:true exists exactly to run prompting commands, so the
// non-interactive guard steps aside and the PTY path gets the session.
if !ttyRequested {
if interactive := zeroSandbox.DetectInteractiveCommand(commandText, runtimeGOOS()); interactive.Interactive {
result := interactiveBlockResult(interactive)
result.Output += "\nRerun with tty:true if the user should interact with it."
return result
}
}
absoluteCwd, relativeCwd, err := resolveScopedPath(tool.workspaceRoot, tool.scope, workdir)
if err != nil {
Expand Down Expand Up @@ -453,7 +474,7 @@ func execToolResultWithBudget(input execToolResultInput, directBudget bool) Resu
if input.exited && input.exitCode != 0 && !input.interrupted {
status = StatusError
}
body := formatExecCommandOutput(output, input.sessionID, input.exited, input.exitCode, input.interrupted)
body := formatExecCommandOutput(output, input.sessionID, input.exited, input.exitCode, input.interrupted, input.tty)
if status == StatusError && input.exited && !input.interrupted {
if issue := detectShellOutputIssueForRuntime(output, detectShellRuntime(runtimeGOOS())); issue != nil {
meta["shell_issue"] = issue.Kind
Expand Down Expand Up @@ -575,7 +596,7 @@ func executionChangeSummaries(changes []execution.Change) []execution.Change {
return summaries
}

func formatExecCommandOutput(output string, sessionID int, exited bool, exitCode int, interrupted bool) string {
func formatExecCommandOutput(output string, sessionID int, exited bool, exitCode int, interrupted bool, tty bool) string {
output = strings.TrimRight(output, "\r\n")
parts := []string{}
if output != "" {
Expand All @@ -593,12 +614,18 @@ func formatExecCommandOutput(output string, sessionID int, exited bool, exitCode
parts = append(parts, "interrupted: true")
}
parts = append(parts, fmt.Sprintf("exit_code: %d", exitCode))
if exitCode != 0 && strings.Contains(output, "no new privileges") {
parts = append(parts, `Hint: this command needs sandbox_permissions "require_escalated" (the sandbox blocks setuid); retry with it and tty:true if it prompts.`)
}
} else {
if output == "" {
parts = append(parts, "Command is still running.")
}
parts = append(parts, fmt.Sprintf("session_id: %d", sessionID))
parts = append(parts, fmt.Sprintf("Use write_stdin with session_id %d and empty chars to poll; send chars \"\\u0003\" to interrupt/stop it.", sessionID))
if tty {
parts = append(parts, fmt.Sprintf("This session has a terminal and the user can already type into it (it opened in their TUI; /attach %d reopens it). If it is waiting on a password or confirmation, say so in one line, then keep polling with write_stdin (empty chars, yield_time_ms 60000) until it exits. Do not end your turn while it is running, and never ask for the password in chat.", sessionID))
}
}
return strings.Join(parts, "\n")
}
Expand Down
53 changes: 53 additions & 0 deletions internal/tools/exec_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -894,3 +894,56 @@ func TestTruncateExecOutputPreservesUTF8(t *testing.T) {
t.Fatalf("truncated output is not valid UTF-8: %q", truncated)
}
}

func TestFormatExecCommandOutputTTYAttachHint(t *testing.T) {
running := formatExecCommandOutput("", 1007, false, 0, false, true)
if !strings.Contains(running, "/attach 1007 reopens it") || !strings.Contains(running, "Do not end your turn while it is running") {
t.Fatalf("tty running session should point at /attach and keep-poll guidance: %q", running)
}
exited := formatExecCommandOutput("", 1007, true, 0, false, true)
if strings.Contains(exited, "/attach") {
t.Fatalf("exited session should not mention /attach: %q", exited)
}
pipes := formatExecCommandOutput("", 1007, false, 0, false, false)
if strings.Contains(pipes, "/attach") {
t.Fatalf("non-tty session should not mention /attach: %q", pipes)
}
}

func TestExecCommandInteractiveBlockSuggestsTTY(t *testing.T) {
tool := NewScopedExecCommandTool(t.TempDir(), nil, newExecSessionManager())
result := tool.Run(context.Background(), map[string]any{"cmd": "ssh host.example.com"})
if result.Status != StatusError || result.Meta["safety_block"] != "interactive_command" {
t.Fatalf("expected interactive safety block, got meta=%#v output=%q", result.Meta, result.Output)
}
if !strings.Contains(result.Output, "tty:true") {
t.Fatalf("block output should suggest tty:true: %q", result.Output)
}
}

func TestExecCommandTTYSkipsInteractiveBlock(t *testing.T) {
tool := NewScopedExecCommandTool(t.TempDir(), nil, newExecSessionManager())
// ssh to a bogus host fails fast on its own; the point is that the
// interactive guard must not fire ahead of the tty path.
result := tool.Run(context.Background(), map[string]any{
"cmd": "ssh host.example.com", "tty": true, "yield_time_ms": 250,
})
if result.Meta["safety_block"] == "interactive_command" {
t.Fatalf("tty:true should not hit the interactive block: %#v", result.Meta)
}
}

func TestFormatExecCommandOutputNoNewPrivilegesHint(t *testing.T) {
blocked := formatExecCommandOutput(`sudo: The "no new privileges" flag is set`, 1007, true, 1, false, true)
if !strings.Contains(blocked, `sandbox_permissions "require_escalated"`) {
t.Fatalf("no_new_privs failure should hint at require_escalated: %q", blocked)
}
normal := formatExecCommandOutput("some other error", 1007, true, 1, false, true)
if strings.Contains(normal, "require_escalated") {
t.Fatalf("unrelated failure should not hint at require_escalated: %q", normal)
}
blockedOK := formatExecCommandOutput(`sudo: The "no new privileges" flag is set`, 1007, true, 0, false, true)
if strings.Contains(blockedOK, "require_escalated") {
t.Fatalf("zero-exit output should not hint at require_escalated: %q", blockedOK)
}
}
2 changes: 1 addition & 1 deletion internal/tui/attachment_preview.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (m model) attachmentThumbnailVisible(width int) bool {
}
if m.pendingPermission != nil || m.pendingAskUser != nil || m.pendingSpecReview != nil ||
m.providerWizard != nil || m.mcpAddWizard != nil || m.mcpManager != nil || m.picker != nil ||
m.sttKeyPrompt != nil || m.renamePrompt != nil || m.setup.visible || m.helpOverlay || m.leaderHelpOverlay {
m.sttKeyPrompt != nil || m.renamePrompt != nil || m.terminalAttach != nil || m.setup.visible || m.helpOverlay || m.leaderHelpOverlay {
return false
}
for index := 0; index < m.attachmentThumbnailSlots(width); index++ {
Expand Down
Loading
Loading