feat: implement terminal streaming with diff algorithm#113
feat: implement terminal streaming with diff algorithm#1132witstudios wants to merge 3 commits intomainfrom
Conversation
Per-agent terminal output streaming with efficient longest-common-suffix diff algorithm that only sends new lines, not the full buffer. - Per-agent subscriptions with lazy initialization (polling starts on first subscriber, stops when last unsubscribes) - 500ms polling interval for tmux pane content via capturePane() - Longest common suffix diff — finds overlap between previous and current buffer snapshots to emit only new lines - Shared timer across clients watching the same agent (single capture per interval regardless of subscriber count) - Auto-cleanup when subscriber count drops to 0 - Error handling for dead/missing panes with subscriber notification - Injectable capture function for testability - 23 tests covering diff algorithm, subscription lifecycle, shared timer, polling behavior, error handling, and cleanup Closes #75
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3794c3912c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/server/ws/terminal.ts
Outdated
| stream.timer = setInterval(() => { | ||
| void this.poll(agentId, stream); | ||
| }, this.pollIntervalMs); |
There was a problem hiding this comment.
startPolling schedules poll via setInterval without waiting for the previous async poll to finish, so if capture takes longer than pollIntervalMs two polls can overlap; an older capture that resolves later can overwrite stream.lastLines with stale data, which then causes duplicated or missing lines in subsequent terminal messages. Guarding with an in-flight flag (or chaining with setTimeout after completion) avoids this ordering corruption.
Useful? React with 👍 / 👎.
src/server/ws/terminal.ts
Outdated
| } catch { | ||
| // Dead client — remove on next tick | ||
| stream.subscribers.delete(sub.id); | ||
| } |
There was a problem hiding this comment.
When sub.send throws, the subscriber is deleted, but there is no follow-up cleanup if that was the final subscriber, leaving an empty stream that continues polling tmux indefinitely. This creates unnecessary background load and keeps stale stream state alive until some later capture error happens; after the send loop, check for stream.subscribers.size === 0 and clear/delete the stream immediately.
Useful? React with 👍 / 👎.
- Switch from setInterval to chained setTimeout to prevent concurrent poll races when capturePane takes longer than the poll interval - Replace loose equality (!=) with strict equality (===) in isPolling - Remove unused loop variable in destroy() - Log original error in catch block for debugging (console.error) - Add test for double-unsubscribe idempotency - Add test for trailing empty lines from tmux capturePane output - Verify error logging in pane-failure test
1 participant
Summary
capturePanecall per tick)Files
src/server/ws/terminal.ts—TerminalStreamerclass,diffLines()pure function,SendFninterfacesrc/server/ws/terminal.test.ts— 23 tests covering diff algorithm, subscription lifecycle, shared timer, polling, error handling, and cleanupTest plan
diffLines— empty inputs, identical buffers, appended lines, scrolled buffer, complete rewrite, partial overlapCloses #75