Skip to content

feat: add resize_pty protocol message for remote PTY resize#575

Merged
willwashburn merged 3 commits intomainfrom
feat/resize-pty
Mar 17, 2026
Merged

feat: add resize_pty protocol message for remote PTY resize#575
willwashburn merged 3 commits intomainfrom
feat/resize-pty

Conversation

@willwashburn
Copy link
Copy Markdown
Member

@willwashburn willwashburn commented Mar 16, 2026

Summary

  • Adds resize_pty protocol message across the full stack: Rust protocol types, broker dispatch, PTY worker handler, and TypeScript SDK client/types
  • When PTY agents are spawned by SDK clients (e.g. Electron apps embedding xterm.js), the worker process has no terminal attached so SIGWINCH-based resize never fires — the PTY stays at its default 80x24
  • New resizePty(name, rows, cols) SDK method lets clients send terminal dimensions to the broker, which forwards them to the worker, which calls pty.resize()

Changes

  • src/protocol.rs: Add ResizePty { rows, cols } to BrokerToWorker enum
  • src/main.rs: Add resize_pty handler in broker dispatch — validates agent exists, forwards to worker via send_to_worker, returns confirmation
  • src/pty_worker.rs: Handle resize_pty message in worker select loop — calls pty.resize(rows, cols)
  • packages/sdk/src/protocol.ts: Add resize_pty to SdkToBroker and BrokerToWorker types
  • packages/sdk/src/client.ts: Add resizePty() method to AgentRelayClient

Test plan

  • cargo check passes
  • tsc --noEmit passes
  • Existing protocol round-trip tests pass
  • Manual test: spawn a PTY agent from an Electron app, call resizePty() on xterm resize, verify the agent TUI re-renders at the correct dimensions

🤖 Generated with Claude Code


Open with Devin

When PTY agents are spawned by SDK clients (e.g. Electron apps), the
worker process has no terminal attached, so SIGWINCH-based resize
doesn't work. This adds a resize_pty message that flows from SDK →
broker → worker, allowing clients to resize the PTY when their
terminal emulator dimensions change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@barryonthecape barryonthecape left a comment

Choose a reason for hiding this comment

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

Nice addition overall — wiring resize_pty through SDK → broker → worker is the right approach. I'm requesting changes for one behavior that can cause hard-to-debug terminal state issues.

Blocking issue:

  • In src/pty_worker.rs, the new resize_pty handler silently falls back to rows=24 / cols=80 when payload fields are missing or invalid:
    • frame.payload.get("rows")...unwrap_or(24)
    • frame.payload.get("cols")...unwrap_or(80)

Why this is risky:

  • A malformed frame can unexpectedly snap an active session back to 80x24 instead of failing fast.
  • The broker already deserializes strongly typed u16 values for resize_pty; introducing permissive defaults in the worker creates inconsistent validation across layers.
  • This can hide protocol/client bugs and make UI resize issues intermittent.

Requested fix:

  1. Parse rows/cols strictly in the worker path (or decode as BrokerToWorker::ResizePty), and reject invalid payloads instead of defaulting.
  2. Validate non-zero dimensions (rows > 0 && cols > 0) before calling pty.resize.
  3. Return or emit a structured worker error for invalid resize payloads so clients can surface actionable diagnostics.

After that, this looks good to me.

This comment was marked as resolved.

willwashburn and others added 2 commits March 16, 2026 14:22
- Reject headless agents with unsupported_operation error
- Validate rows/cols >= 1, return invalid_dimensions error for zeros
- Use typed struct deserialization in pty_worker instead of manual
  JSON field extraction with silent defaults
- Add BrokerToWorker::ResizePty round-trip test with wire format checks

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@willwashburn
Copy link
Copy Markdown
Member Author

@barryonthecape All three points addressed in 511cff4:

  1. Strict deserialization — worker now uses `serde_json::from_value::` instead of manual field extraction with silent defaults. Invalid payloads return a `worker_error` with `invalid_payload` code.
  2. Non-zero validation — both broker and worker reject `rows == 0 || cols == 0` with an `invalid_dimensions` error.
  3. Structured error responses — worker sends typed `worker_error` frames for both invalid payloads and invalid dimensions so clients get actionable diagnostics.

Also added: broker rejects `resize_pty` for headless agents with `unsupported_operation`, and a round-trip test for the wire format.

Copy link
Copy Markdown
Contributor

@barryonthecape barryonthecape left a comment

Choose a reason for hiding this comment

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

Thanks for the fast follow-up — requested changes are addressed.\n\nVerified on latest commits:\n- Worker now strictly deserializes resize payloads (no silent 80x24 fallback) and returns structured worker_error invalid_payload on malformed input.\n- Broker and worker both reject zero dimensions with explicit invalid_dimensions errors.\n- Broker now rejects resize_pty for non-PTY agents with unsupported_operation.\n- Added protocol round-trip coverage for BrokerToWorker::ResizePty wire format.\n\nLGTM.

@willwashburn willwashburn merged commit 6f6c097 into main Mar 17, 2026
1 check passed
@willwashburn willwashburn deleted the feat/resize-pty branch March 17, 2026 14:19
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.

3 participants