fix: terminal input and cleanup

[joshuapare]

Summary

• Base64-encode terminal input before calling WriteSession — Wails v3 serializes []byte as base64 over JSON, but the frontend was sending raw keystrokes, causing every keystroke to fail with illegal base64 data at input byte 0
• Reduce terminal debug log noise — remove raw ANSI buffer dumps, per-keystroke input logs, and debug breadcrumbs
• Centralize signal handling — single Manager-level goroutine forwards host-process signals to all child terminals instead of one listener per session competing for signals
• Fix ListSessions lock — use RLock instead of Lock for read-only operation
• Remove dead code — unused cleanPTYOutput and listenOnOut functions

Test plan

• Open a local terminal in Omniview — verify keystrokes are accepted and output renders
• Open multiple terminals simultaneously — verify signals don't interfere
• Verify plugin exec sessions (e.g. Kubernetes) also accept input
• Check logs are clean without raw ANSI dumps or per-keystroke spam

Summary by CodeRabbit

• Refactor
  • Enhanced terminal session management with optimized signal handling and command tracking
  • Updated terminal input data encoding for consistency

[coderabbitai]

Warning

Rate limit exceeded

@joshuapare has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 28 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d3bd04f1-2d11-4acb-b5cd-1e43e2f1af94

📥 Commits

Reviewing files that changed from the base of the PR and between a3fd899 and ceca022.

📒 Files selected for processing (2)

• backend/pkg/plugin/exec/controller.go
• backend/pkg/terminal/manager.go

📝 Walkthrough

Walkthrough

Backend refactoring shifts terminal session lifecycle management from external cancel channels to context-based control, introduces manager-wide signal forwarding for tracked commands, and removes debug logging. Frontend adds Base64 encoding for keystroke data before transmission.

Changes

Cohort / File(s)	Summary
Control Flow Context Refactoring backend/pkg/plugin/exec/controller.go, backend/pkg/terminal/manager.go, backend/pkg/terminal/manager_windows.go	Replaces per-session external cancellation via listenOnOut() goroutine with context-based lifecycle management. terminal.Manager now accepts context.Context, stores it, and manages session termination through context cancellation. Signal forwarding refactored from per-session handlers to manager-wide iteration over tracked commands.
Session Tracking & Signal Handling backend/pkg/terminal/manager.go	Introduces cmds map[string]*exec.Cmd to track active session commands. Implements forwardSignals() goroutine that broadcasts SIGTERM/SIGINT/SIGQUIT to all tracked commands. Replaces per-session handleSignals() with handleSessionClose() that detects session context cancellation and emits StreamSignalClose. Cleanup logic removes command entries in terminateSessionLocked().
Debugging & Logging Adjustments backend/pkg/plugin/exec/controller.go, backend/pkg/terminal/manager.go	Removes debug log for incoming plugin stream inputs ("got input"). Updates logging in StartSession and AttachSession to include command/TTY and session/buffer metadata. Changes ListSessions from Lock to RLock and logs session count instead of individual sessions.
UI Keystroke Encoding ui/providers/BottomDrawer/containers/Terminal.tsx	Encodes keystroke payload with Base64 before calling ExecClient.WriteSession(), changing from raw data to Base64.encode(data).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 With whiskers twitching, I spy the change,
No more loose threads in signals' range!
Context commands, tracked so neat,
Base64 keystrokes, Base-encoded and sweet!
Manager goroutines now forward with care,
A cleanup so proper—quite rare! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: terminal input and cleanup' accurately reflects the main changes: Base64-encoding terminal input, refactoring signal handling, and removing debug noise.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/terminal-input-and-cleanup

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/pkg/plugin/exec/controller.go`:
- Around line 120-122: The controller currently initializes
terminal.NewManager(...) inside runLocalMux and assigns c.terminalManager there,
causing unsynchronized reads (nil/datarace) from methods like CreateSession,
ListSessions, GetSession, AttachSession, DetachSession, WriteSession,
CloseSession, and ResizeSession; instead, call terminal.NewManager(c.ctx,
c.logger) in ServiceStartup, assign the resulting manager to c.terminalManager
before spawning the goroutine, and pass the returned inMux, outMux, resizeMux
into runLocalMux as parameters so runLocalMux uses those channels but does not
perform the manager assignment (move creation/assignment out of
controller.runLocalMux into ServiceStartup and change runLocalMux signature to
accept inMux, outMux, resizeMux).

In `@backend/pkg/terminal/manager.go`:
- Around line 35-36: The session contexts are rooted in context.Background() so
shutting down m.ctx doesn't cancel active PTYs; update the code that creates
each session (the spot that currently uses context.Background() — referenced
around the session/spawn logic and line creating the session context) to derive
from the manager context (use m.ctx or context.WithCancel(m.ctx)) instead of
context.Background(), ensuring forwardSignals() exiting on m.ctx.Done() will
cancel sessions and trigger handleSessionClose; also verify any goroutines that
run per-session accept that derived context and propagate it to the PTY
lifecycle and handleSessionClose invocation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7dd2f5be-f43a-4708-abc5-2c81a17a00f5

📥 Commits

Reviewing files that changed from the base of the PR and between dd2bd55 and a3fd899.

📒 Files selected for processing (4)

• backend/pkg/plugin/exec/controller.go
• backend/pkg/terminal/manager.go
• backend/pkg/terminal/manager_windows.go
• ui/providers/BottomDrawer/containers/Terminal.tsx