refactor(server): effectify pty json route handlers#1015
Conversation
|
Warning Review limit reached
More reviews will be available in 50 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the PTY routes in packages/opencode/src/server/instance/pty.ts to utilize the Effect library and AppRuntime.runPromise for managing PTY operations (list, create, get, update, and remove) instead of direct asynchronous calls on static Pty methods. There are no review comments, and I have no feedback to provide.
Summary
Effectify the ordinary JSON PTY route handlers while leaving the WebSocket connection path unchanged:
AppRuntime.runPromise(Effect.gen(...));Pty.Servicefrom the runtime context instead of calling the async facade helpers;Why
Part of #936. The route handler effectification phase should move small, ordinary JSON handlers onto the shared Effect runtime before any larger route-group or HttpApi migration. PTY has focused route coverage and a clear boundary between JSON handlers and the WebSocket connect path.
Related Issue
Part of #936.
Human Review Status
Pending
Review Focus
Please review whether the JSON handlers preserve the old PTY service call order and whether the WebSocket/connect-token boundary is truly unchanged.
Risk Notes
How To Verify
Screenshots or Recordings
Not applicable; no visible UI changes.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.