Skip to content

fix(broker): handle write_pty frames in PTY worker#921

Merged
willwashburn merged 1 commit into
mainfrom
claude/fix-issue-920-fGMJ4
May 19, 2026
Merged

fix(broker): handle write_pty frames in PTY worker#921
willwashburn merged 1 commit into
mainfrom
claude/fix-issue-920-fGMJ4

Conversation

@willwashburn
Copy link
Copy Markdown
Member

Summary

Fixes #920.

AgentRelayClient.sendInput() / POST /api/input/{name} returned success for PTY workers but the bytes never reached the child process. The HTTP route forwards a write_pty frame via workers.send_to_worker(..., "write_pty", ...) (crates/broker/src/runtime/api.rs:893), but crates/broker/src/pty_worker.rs only handled resize_pty and snapshot_ptywrite_pty fell through to the catch-all and emitted worker_error: unsupported message type 'write_pty'. Interactive clients (e.g. Pear's terminal tabs attaching to a spawned Claude/Codex PTY) could not type into the terminal.

Change

Add a write_pty arm to the PTY worker's stdin frame handler in crates/broker/src/pty_worker.rs that:

  • Pulls the data string from the payload (matching runtime/api.rs's json!({ "data": data }) shape).
  • Calls pty.write_all(data.as_bytes()) to push the keystrokes through the existing write queue.
  • Emits a worker_error frame only on malformed payloads (invalid_payload) or PTY write failures (pty_write_failed, retryable).

The HTTP route is already correct — this just teaches the worker to honor the frame the route is already sending.

Test plan

  • cargo build -p agent-relay-broker succeeds.
  • cargo test -p agent-relay-broker --lib pty — all 57 PTY tests pass.
  • Manual repro from the issue body: spawn a PTY agent, call client.sendInput(name, 'printf READY\n\r'), confirm the snapshot now contains READY and no worker_error event is emitted.

Generated by Claude Code

The HTTP `/api/input/{name}` route forwards a `write_pty` frame to the
PTY worker, but the worker only handled `resize_pty` and `snapshot_pty`,
so the frame fell through to the unknown-type branch and the bytes
never reached the child PTY — even though the HTTP caller saw a
successful response with `bytes_written`. Interactive clients (e.g.
Pear's terminal tabs) attached via `sendInput()` could not type into
the spawned Claude/Codex PTYs.

Add a `write_pty` handler that pulls the `data` string from the payload
and writes it through `pty.write_all`, emitting a `worker_error` frame
only on malformed payloads or PTY write failures.

Fixes #920
@willwashburn willwashburn requested a review from khaliqgant as a code owner May 19, 2026 19:43
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dba3246d-e5cc-4582-870e-daeab1c38b4f

📥 Commits

Reviewing files that changed from the base of the PR and between def39b0 and 398a78f.

📒 Files selected for processing (1)
  • crates/broker/src/pty_worker.rs

📝 Walkthrough

Walkthrough

The PTY worker now accepts write_pty inbound messages from the broker's HTTP /api/input/{name} endpoint. When received, the handler extracts UTF-8 data from the payload, writes it directly to the child PTY process, and returns appropriate error frames for missing data fields or write failures.

Changes

PTY Write Input Support

Layer / File(s) Summary
write_pty message handler
crates/broker/src/pty_worker.rs
The run_pty_worker function now handles write_pty messages: parses the UTF-8 data field from the payload, writes it to the PTY, and emits non-retryable worker_error frames for missing data or retryable errors on write failures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • khaliqgant

Poem

🐰 A write_pty handler, oh what a treat,
Now input flows where agents and humans meet!
No more "unsupported" errors to lament,
The PTY worker speaks the protocol's intent.
hop hop

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(broker): handle write_pty frames in PTY worker' clearly and concisely summarizes the main change—adding write_pty frame handling to the PTY worker.
Description check ✅ Passed The description comprehensively explains the problem, solution, and testing approach. It references the linked issue, details why write_pty was falling through unhandled, and documents the implementation and test coverage.
Linked Issues check ✅ Passed The changes fully address issue #920 by implementing the write_pty frame handler that extracts the data payload, writes to the PTY, and emits appropriate error frames, resolving the broken interactive API client scenario.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the stated objective: adding write_pty frame handling in the PTY worker with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-issue-920-fGMJ4

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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 2 additional findings.

Open in Devin Review

@willwashburn willwashburn merged commit d18f687 into main May 19, 2026
40 checks passed
@willwashburn willwashburn deleted the claude/fix-issue-920-fGMJ4 branch May 19, 2026 19:51
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.

PTY sendInput returns success but worker rejects write_pty frame

2 participants