Skip to content

feat(tools): add exec tool enhancement with background execution and PTY support#1752

Merged
yinwm merged 1 commit intosipeed:mainfrom
liuy:feat/exec-tool-enhancement
Mar 21, 2026
Merged

feat(tools): add exec tool enhancement with background execution and PTY support#1752
yinwm merged 1 commit intosipeed:mainfrom
liuy:feat/exec-tool-enhancement

Conversation

@liuy
Copy link
Contributor

@liuy liuy commented Mar 18, 2026

Background

Before this patch, picoclaw's exec tool only supports synchronous execution - it blocks and waits for the command to complete before returning. This limits the agent from:

  • Running long-running commands (docker build, cargo build)
  • Handling interactive input (sudo passwords, TUI apps)
  • Managing persistent processes

Core Logic Comparison

┌─────────────────────────────────────────────────────────────────────┐
│                    BEFORE (without this patch)                      │
├─────────────────────────────────────────────────────────────────────┤
│                                                                     │
│  Execute(command)                                                   │
│       │                                                             │
│       ▼                                                             │
│  ┌─────────┐                                                        │
│  │ runSync │◄── blocks until command completes                      │
│  └────┬────┘                                                        │
│       │                                                             │
│       ▼                                                             │
│  returns result (complete)                                          │
│                                                                     │
│  ❌ No background execution                                         │
│  ❌ No PTY support                                                  │
│  ❌ No session concept                                              │
│                                                                     │
└─────────────────────────────────────────────────────────────────────┘

┌─────────────────────────────────────────────────────────────────────┐
│                    AFTER (with this patch)                          │
├─────────────────────────────────────────────────────────────────────┤
│                                                                     │
│  Execute(action, command, background?, pty?)                        │
│       │                                                             │
│       ├──────────────────────────────────────────────┐              │
│       │                                              │              │
│   action=run                                    action≠run           │
│       │                                              │              │
│       ▼                                              ▼              │
│  background=true?                              poll/read/            │
│       │                                         write/kill           │
│   ┌───┴───┐                                         │              │
│   yes     no                                         │              │
│   │       │                                          │              │
│   ▼       ▼                                          │              │
│ runBg   runSync                               operate on SessionMgr  │
│   │       │                                          │              │
│   ▼       ▼                                          │              │
│ session + sessionId ──────────────────────────► lookup session      │
│   │                                                         │       │
│   ▼                                                         ▼       │
│ returns immediately ✅                               returns status  │
│                                              ✅ non-blocking        │
│                                                                     │
│  ✅ Background execution (background=true)                          │
│  ✅ PTY support (pty=true, for interactive input like sudo)         │
│  ✅ SessionManager manages all running processes                    │
│                                                                     │
└─────────────────────────────────────────────────────────────────────┘

Changes

New Files

  • pkg/tools/session.go - SessionManager, ProcessSession
  • pkg/tools/session_process_unix.go - Unix process group kill (Setpgid + SIGKILL)
  • pkg/tools/session_process_windows.go - Windows fallback
  • pkg/tools/session_test.go - session management tests

Modified Files

  • pkg/tools/shell.go - action routing (run/list/poll/read/write/kill)
  • pkg/tools/types.go - ExecRequest/ExecResponse types
  • pkg/tools/shell_test.go - new tests
  • go.mod - added creack/pty dependency

Tool Description Enhancements

Improved exec tool descriptions for better agent usability:

  • poll action: Added return format {sessionId, status: running|done, exitCode}
  • read action: Added return format {sessionId, output, status: running|done}
  • timeout default: Changed from 60s to 0 (no timeout)
  • keys separator: Added clarification "(optional spaces around comma)"
  • Session lifecycle: Added auto-cleanup 30 min, output buffer 100MB
  • pty+background: Added "(can combine with background=true)"
  • write action: Added "(only when status=running)"

Local Testing

All tests passed locally:

  • go test ./... - All packages pass
  • golangci-lint run ./... - No issues

Test Case: Snake Game with PTY

Agent created a snake game using curses and played it via PTY session:

#!/usr/bin/env python3
"""贪吃蛇 - LLM 友好版,输出纯文本网格,使用 curses 处理输入"""

import curses
import random

# 游戏参数
WIDTH = 20
HEIGHT = 15
FRAME_MS = 1500

def render(snake, food, score, direction):
    """渲染游戏画面为纯文本网格"""
    # 创建空网格
    grid = [['.' for _ in range(WIDTH)] for _ in range(HEIGHT)]
    
    # 绘制边界
    for x in range(WIDTH):
        grid[0][x] = '#'
        grid[HEIGHT-1][x] = '#'
    for y in range(HEIGHT):
        grid[y][0] = '#'
        grid[y][WIDTH-1] = '#'
    
    # 绘制蛇
    for i, (y, x) in enumerate(snake):
        if i == 0:
            grid[y][x] = '@'  # 蛇头
        else:
            grid[y][x] = 'o'  # 蛇身
    
    # 绘制食物
    grid[food[0]][food[1]] = '*'
    
    # 方向翻译
    dir_map = {
        'up': '↑ 上',
        'down': '↓ 下',
        'left': '← 左',
        'right': '→ 右'
    }
    dir_display = dir_map.get(direction, direction)
    
    output = []
    output.append(f"=== 贪吃蛇游戏 | 得分: {score} | 方向: {dir_display} ===")
    output.append("")
    for row in grid:
        output.append(' '.join(row))
    output.append("")
    output.append("蛇头: @  蛇身: o  食物: *  空地: .  墙: #")
    output.append("控制: 方向键=移动 q=退出")
    output.append("")
    output.append(f"蛇头位置: ({snake[0][1]}, {snake[0][0]})")
    output.append(f"食物位置: ({food[1]}, {food[0]})")
    output.append(f"蛇长度: {len(snake)}")
    
    return '\n'.join(output)

def main(stdscr):
    # 设置非阻塞输入
    stdscr.nodelay(True)
    
    # 初始化蛇
    snake = [(5, 7)]  # 只有蛇头
    food = (3, 10)
    direction = 'right'
    score = 0
    
    while True:
        # 渲染画面(清屏 + 输出)
        stdscr.clear()
        stdscr.addstr(0, 0, render(snake, food, score, direction))
        stdscr.refresh()
        
        # 等待按键(非阻塞,每 50ms 检查一次)
        new_direction = direction
        for _ in range(FRAME_MS // 50):
            key = stdscr.getch()
            if key == ord('q'):
                return
            # 检测 curses.KEY_UP/DOWN/LEFT/RIGHT(依赖 terminfo)
            elif key == curses.KEY_UP and direction != 'down':
                new_direction = 'up'
            elif key == curses.KEY_DOWN and direction != 'up':
                new_direction = 'down'
            elif key == curses.KEY_LEFT and direction != 'right':
                new_direction = 'left'
            elif key == curses.KEY_RIGHT and direction != 'left':
                new_direction = 'right'
            
            curses.napms(50)  # 等待 50ms
        
        direction = new_direction
        
        # 计算新头部位置
        head_y, head_x = snake[0]
        if direction == 'up':
            head_y -= 1
        elif direction == 'down':
            head_y += 1
        elif direction == 'left':
            head_x -= 1
        elif direction == 'right':
            head_x += 1
        
        # 检查碰撞
        if (head_y <= 0 or head_y >= HEIGHT-1 or
            head_x <= 0 or head_x >= WIDTH-1 or
            (head_y, head_x) in snake):
            stdscr.clear()
            stdscr.addstr(0, 0, "游戏结束!")
            stdscr.addstr(1, 0, f"最终得分: {score}")
            stdscr.refresh()
            curses.napms(2000)
            break
        
        # 移动蛇
        snake.insert(0, (head_y, head_x))
        
        # 检查吃到食物
        if (head_y, head_x) == food:
            score += 10
            while True:
                food = (random.randint(1, HEIGHT-2), random.randint(1, WIDTH-2))
                if food not in snake:
                    break
        else:
            snake.pop()

if __name__ == "__main__":
    curses.wrapper(main)

Agent execution flow:

  1. exec action=run command="python3 snake_curses.py" background=true pty=true → sessionId
  2. exec action=read sessionId=xxx → see game screen
  3. exec action=send-keys sessionId=xxx keys="right, down, left, up" → control snake
  4. exec action=kill sessionId=xxx → end game

This demonstrates PTY support for interactive TUI applications.

Related Issue

Fixes #1733

@liuy liuy marked this pull request as draft March 18, 2026 15:32
@liuy liuy force-pushed the feat/exec-tool-enhancement branch 3 times, most recently from 10e550c to b2fcc59 Compare March 19, 2026 10:28
@sipeed-bot sipeed-bot bot added type: enhancement New feature or request domain: tool dependencies Pull requests that update a dependency file labels Mar 19, 2026
@liuy liuy force-pushed the feat/exec-tool-enhancement branch 3 times, most recently from 0eeae2d to 6753aee Compare March 19, 2026 16:05
@liuy liuy marked this pull request as ready for review March 19, 2026 16:06
@liuy liuy force-pushed the feat/exec-tool-enhancement branch 4 times, most recently from df859bd to 544b4ab Compare March 21, 2026 13:59
@yinwm
Copy link
Collaborator

yinwm commented Mar 21, 2026

Code Review: ExitCode Handling Issue

I found one issue while reviewing this PR:

[CRITICAL] ExitCode is not properly captured

Location: pkg/tools/shell.go:566-573 and pkg/tools/shell.go:613-619

Problem: The current implementation only sets ExitCode to 0 or 1 based on cmd.ProcessState.Success(), losing the actual exit code:

if cmd.ProcessState.Success() {
    session.ExitCode = 0
} else {
    session.ExitCode = 1  // ❌ Should be cmd.ProcessState.ExitCode()
}

Impact: Cannot distinguish between different failure modes:

  • OOM killed → exit code 137
  • Segmentation fault → exit code 139
  • Signal killed → exit code -1
  • Custom exit codes → lost

Fix: Use cmd.ProcessState.ExitCode() to capture the actual exit code:

session.ExitCode = cmd.ProcessState.ExitCode()

Other minor observations (acceptable for this use case):

  • Goroutine in NewSessionManager() runs for process lifetime - acceptable since it's a global singleton
  • API change working_dircwd - acceptable since primarily LLM Agent usage
  • 8-char session ID has 32-bit entropy - acceptable for expected concurrency levels

Overall: Great feature addition! The background execution and PTY support are well-designed.

…PTY support

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup
@liuy liuy force-pushed the feat/exec-tool-enhancement branch from 544b4ab to 62630f9 Compare March 21, 2026 14:29
@liuy
Copy link
Contributor Author

liuy commented Mar 21, 2026

Thanks for the review! Fixed the ExitCode issue - now using cmd.ProcessState.ExitCode() to capture the actual exit code. This correctly distinguishes:

  • OOM killed (137)
  • Segmentation fault (139)
  • Signal killed (-1)
  • Custom exit codes

Copy link
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

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

LGTM! The ExitCode issue has been fixed. Ready to merge.

@yinwm yinwm merged commit f901af8 into sipeed:main Mar 21, 2026
4 checks passed
liuy added a commit to liuy/picoclaw that referenced this pull request Mar 21, 2026
…PTY support (sipeed#1752)

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup
liuy added a commit to liuy/picoclaw that referenced this pull request Mar 21, 2026
…PTY support (sipeed#1752)

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup
liuy added a commit to liuy/picoclaw that referenced this pull request Mar 25, 2026
…PTY support (sipeed#1752)

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup
samueltuyizere added a commit to samueltuyizere/picoclaw that referenced this pull request Mar 25, 2026
* update security migration documents

* feat(config): add command pattern detection tool in exec settings (sipeed#1971)

* Add command pattern testing endpoint and UI tool

Adds a new API endpoint `/api/config/test-command-patterns` that tests a
command against configured whitelist and blacklist patterns, along with
a frontend UI component to interactively test patterns.

* Only process deny patterns when enableDenyPatterns is true

* feat(models): add extra_body config field in model add/edit UI (sipeed#1969)

* Add extraBody field to model configuration forms

This adds a new field allowing users to specify additional JSON fields
to inject into the request body when configuring models.

* Handle ExtraBody clearing when frontend sends empty object

The backend now interprets an empty object sent from the frontend as a
signal to clear the ExtraBody field, while nil/undefined preserves the
existing value. Frontend changed to send {} instead of undefined when
the field is empty.

* refactor(web): clean up systray platform build files

Separate embedded tray icons into platform-specific files, rename the
no-cgo systray stub for consistency, and add the app version to the
launcher startup log.

* fix(agent): suppress heartbeat tool feedback (sipeed#1937)

* fix(lint): remove CGO_ENABLED=0 for lint and fix (sipeed#1989)

* fix(lint): remove CGO_ENABLED=0 for lint and fix

* fix makefile

* config: add baidu_search example to config.example.json (sipeed#1990)

Add Baidu Qianfan AI Search configuration block after glm_search,
matching the BaiduSearchConfig struct defaults (enabled: false,
max_results: 10).

Co-authored-by: BeaconCat <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>

* Fix security config precedence during migration (sipeed#1984)

* Fix security config precedence during migration

* add doc

* fix ci

* add baidu search

* feat(web): add WeCom QR binding flow to channel settings (sipeed#1994)

- add backend WeCom QR flow endpoints and in-memory flow state management
- add frontend WeCom binding UI with QR polling and channel enable toggle
- update channel config behavior and i18n strings for WeCom and WeChat
- apply minor formatting cleanup in model-related components

* chore(tui): add build target for picoclaw-launcher TUI and create README for TUI launcher (sipeed#1995)

* fix(build): disable Matrix gateway import on freebsd/arm

Exclude the Matrix gateway shim from freebsd/arm builds because
modernc.org/libc currently fails to compile on that target.
Document the upstream 32-bit FreeBSD codegen mismatch as well.

* fix(release): ignore nightly tags in goreleaser changelog (sipeed#1999)

GoReleaser was picking nightly tags as the "previous tag" when
generating changelogs, causing release changelogs to be incomplete.
Add git.ignore_tags to skip nightly tags.

* feat(tools): add exec tool enhancement with background execution and PTY support (sipeed#1752)

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup

* docs: update WeChat community QR code (sipeed#2003)

Co-authored-by: BeaconCat <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>

* feat(logger): add PICOCLAW_LOG_FILE env var for file-only logging

* Feature/add mimo provider (sipeed#1987)

* feat: add Xiaomi MiMo provider support

- Add 'mimo' protocol prefix support in factory_provider.go
- Add default API base URL for MiMo: https://api.xiaomimimo.com/v1
- Update provider-label.ts to include Xiaomi MiMo label
- Add MiMo to provider tables in both English and Chinese documentation
- Add comprehensive unit tests for MiMo provider

MiMo API is compatible with OpenAI API format, making it easy to integrate
with the existing HTTPProvider infrastructure.

Users can now use MiMo by configuring:
{
  "model_name": "mimo",
  "model": "mimo/mimo-v2-pro",
  "api_key": "your-mimo-api-key"
}

* hassas dosyaları kaldırma

* Add .security.yml and onboard to .gitignore

* build(deps): upgrade pty and reorganize sqlite dependencies (sipeed#2012)

- Upgrade github.com/creack/pty from v1.1.9 to v1.1.24
- Move github.com/mattn/go-sqlite3 to indirect dependency
- Move rsc.io/qr from indirect to direct dependency

* feat(channels): support multi-message sending via split marker (sipeed#2008)

* Add multi-message sending via split marker

* Add marker and length split integration tests

Tests that SplitByMarker and SplitMessage work together correctly, and
that code block boundaries are preserved during marker splitting.

* Simplify message chunking logic in channel worker

Extract splitByLength helper function and remove goto-based control
flow.
The logic now flows more naturally - try marker splitting first, then
fall
back to length-based splitting.

* Update multi-message output instructions in agent context

* Add split_on_marker to config defaults

* Add split_on_marker config option

* Rename 'Multi-Message Sending' setting to 'Chatty Mode'

* Add SplitOnMarker config option

---------

Co-authored-by: Cytown <[email protected]>
Co-authored-by: 柚子 <[email protected]>
Co-authored-by: wenjie <[email protected]>
Co-authored-by: daming大铭 <[email protected]>
Co-authored-by: xiwuqi <[email protected]>
Co-authored-by: taorye <[email protected]>
Co-authored-by: Luo Peng <[email protected]>
Co-authored-by: BeaconCat <[email protected]>
Co-authored-by: BeaconCat <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: lxowalle <[email protected]>
Co-authored-by: Guoguo <[email protected]>
Co-authored-by: Liu Yuan <[email protected]>
Co-authored-by: 肆月 <[email protected]>
renato0307 pushed a commit to renato0307/picoclaw that referenced this pull request Mar 26, 2026
…PTY support (sipeed#1752)

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file domain: tool type: enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFC: Exec Tool Enhancement - PTY + Background Support

2 participants