Skip to content

feat(shell): add PTY support for interactive commands (sudo, ssh)#1571

Closed
tanikush wants to merge 2 commits into
esengine:mainfrom
tanikush:feat/pty-interactive-input
Closed

feat(shell): add PTY support for interactive commands (sudo, ssh)#1571
tanikush wants to merge 2 commits into
esengine:mainfrom
tanikush:feat/pty-interactive-input

Conversation

@tanikush

Copy link
Copy Markdown

What

Adds PTY (pseudo-terminal) support for interactive shell commands in Reasonix.

Why

Currently interactive commands like sudo, ssh, npm init fail or hang because they require user input via a real terminal.

How

  • Introduced node-pty based execution in pty-exec.ts
  • Added needsPty() detection for interactive commands
  • Integrated PTY path into exec.ts
  • Preserves existing non-interactive spawn flow
  • Safe fallback when process.stdin is not a TTY

##How to verify
Run:
npm run verify

Then test:
sudo echo "test"
ssh localhost (if configured)

Checklist

  • npm run verify passes locally
  • No breaking changes to existing shell execution
  • PTY only used for interactive commands

@esengine

Copy link
Copy Markdown
Owner

Thanks for taking the time to put this together. Unfortunately I don't think we can take this as-is — there are a couple of architectural blockers I should explain so the work isn't wasted.

run_command is invoked by the model, not by a human at the terminal. There's no one to type a sudo password or answer npm init's prompts — the agent loop reads the captured output and decides the next step. So "make interactive commands work" isn't quite the right goal for this tool; the right answer for sudo / ssh is for the model to run them in non-interactive form (sudo -n, ssh -o BatchMode=yes, npm init -y) and surface a clear error when credentials are actually required.

Ink owns stdin/stdout. Reasonix's UI is an Ink TUI, which keeps process.stdin in raw mode and renders on process.stdout. The PTY path here calls process.stdin.setRawMode(true) and attaches its own data listener while Ink is also reading keys — they'll fight over keystrokes — and it writes raw PTY bytes straight to process.stdout, which will corrupt whatever Ink is currently drawing. Any "let the user drive the child process" design has to negotiate with Ink (suspend rendering, hand over the TTY, restore on exit), not bypass it.

A few smaller things I'd flag regardless:

  • opts.signal is dropped on the PTY path, so the parent loop can't cancel it.
  • needsPty uses startsWith on the raw string — sudoku matches sudo, and the prefix list will miss most real interactive tools.
  • Output truncation keeps the tail; the existing exec path keeps the head with a [… truncated …] marker. The model expects the latter shape.
  • node-pty is a native dep with an install script (node-gyp). Adding it raises the install-failure surface, especially on Windows without Build Tools — needs a stronger justification than the current use case.
  • The toLocaleString("en-US") change in src/loop/errors.ts is unrelated to PTY. If you'd like to land that on its own I'm happy to take it as a separate PR.

Going to close this one. Appreciate the contribution — if you're interested in the non-interactive-flag detection angle (helpful error message when a command would have prompted), that'd be a welcome follow-up.

@esengine esengine closed this May 23, 2026
Bernardxu123 pushed a commit to Bernardxu123/DeepSeek-Reasonix that referenced this pull request May 23, 2026
…SessionStats, and card state

Four targeted fixes for memory growth during long sessions:

1. JobRegistry: auto-evict oldest completed jobs when count > 50
2. AppendOnlyLog: auto-compact when entries > 500, keep recent 200
3. SessionStats.turns: rolling window of 200 turns, older costs
   aggregated into carryover
4. state.cards: trim oldest settled cards when count > 150;
   Terminal scrollback retains visual output, only React state freed

Fixes esengine#1571
Bernardxu123 pushed a commit to Bernardxu123/DeepSeek-Reasonix that referenced this pull request May 23, 2026
…SessionStats, and card state

Four targeted fixes for memory growth during long sessions:

1. JobRegistry: auto-evict oldest completed jobs when count > 50
2. AppendOnlyLog: auto-compact when entries > 500, keep recent 200
3. SessionStats.turns: rolling window of 200 turns, older costs
   aggregated into carryover
4. state.cards: trim oldest settled cards when count > 150;
   Terminal scrollback retains visual output, only React state freed

Fixes esengine#1571
@tanikush tanikush deleted the feat/pty-interactive-input branch May 23, 2026 12:13
esengine pushed a commit that referenced this pull request May 23, 2026
- Auto-evict oldest completed jobs when count exceeds MAX_COMPLETED_JOBS (20)
- Cleanup runs on job completion (settleClosed) and list() calls
- No time throttle — gate on count only, O(jobs.size) is trivial at N=20
- Initialize lastCleanupAt to 0 so first call always runs (fixes dead zone)

Fixes #1571 (JobRegistry portion)

Co-authored-by: CI Fix <fix@ci.local>
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