Let the terminal viewer scroll

[lsk567]

The web terminal could not be scrolled.

A tmux client only forwards wheel events when the session asks for mouse tracking, and agent sessions are created without it. So the viewer received nothing to scroll with, and there was no way back through what an agent had already drawn.

Attaching now turns it on for that session. It is best effort — failing to set it costs scrolling, not the attachment.

Side effect worth knowing: with mouse tracking on, clicking and dragging inside the pane is handled by tmux (selection/copy-mode) rather than by the browser. That is the same behaviour as attaching from a terminal with mouse on, and it is what makes the wheel work.

Validation

• cargo test --bin omar — 307 pass, including a new one that turns the option off, attaches, and asserts it comes back on, against real tmux
• cargo fmt --all, cargo clippy --all-targets --all-features -- -D warnings

🤖 Generated with Claude Code

[Copilot]

🟡 Not ready to approve

New tmux commands use non-exact -t session targets that can prefix-match and act on the wrong session, which should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR makes web terminal attachments scrollable by enabling tmux mouse tracking on attach, ensuring wheel events are available for the viewer to scroll through pane history.

Changes:

• Add a best-effort enable_mouse() helper that turns mouse on for the target tmux session.
• Invoke mouse enabling during Attachment::open() before spawning tmux attach-session.
• Add an integration test that creates a real tmux session with mouse off, attaches, and asserts the option becomes on.

File summaries

File	Description
src/terminal.rs	Enables tmux mouse tracking during attachment and adds a tmux-backed regression test to confirm the option is turned on.

Review details

Suppressed comments (2)

src/terminal.rs:316

• This test uses tmux targets like -t session, which can prefix-match and may operate on an unintended session if another session shares the same prefix. Using an exact target (=name) here avoids accidentally killing or mutating the wrong session on a shared tmux server.

        let session = "omar-terminal-mouse-probe";
        let _ = tmux_command()
            .args(["kill-session", "-t", session])
            .output();
        let _guard = SessionGuard(session.to_string());

src/terminal.rs:341

• show-options is also targeted with a prefix match; use the exact =name target computed earlier so the assertion is about the session created by this test.

        let reported = tmux_command()
            .args(["show-options", "-t", session, "-v", "mouse"])
            .output()
            .expect("tmux answers");

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

enable_mouse can permanently toggle session mouse tracking even when Attachment::open fails later, creating unexpected side effects without a successful attachment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/terminal.rs:144

• enable_mouse(&session) runs before several fallible steps (openpty, building/spawning the tmux attach command). If Attachment::open returns an error after enable_mouse succeeds, the session is left with mouse enabled even though no viewer attached, which can change interaction semantics unexpectedly. Consider moving the best-effort enable_mouse call to immediately after spawn_command succeeds so mouse tracking is only enabled when an attachment is actually created.

    pub fn open(prefix: &str, agent: &str) -> Result<Self> {
        let session = format!("{prefix}{}", flatten_agent_name(agent));
        let size = window_size(&session)?;
        enable_mouse(&session);

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

enable_mouse is currently called before remaining fallible PTY setup (so open() can still error after mutating tmux state), and the new “failed attach” test doesn’t actually exercise the intended failure scenario.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/terminal.rs:176

• enable_mouse(&session) is invoked before try_clone_reader() / take_writer() which can still return an error, so Attachment::open can fail while leaving the tmux session modified (mouse tracking enabled) despite the intent expressed in the preceding comment. Move the call to after the remaining fallible PTY setup so the mouse option is only changed when open() is definitely returning Ok.

        // Only now that a viewer really is attached: everything above can fail,
        // and a session left with mouse tracking on by an attempt that never
        // arrived is a change nobody asked for.
        enable_mouse(&session);

src/terminal.rs:356

• This test sets mouse off on session = "omar-terminal-failed-attach" but then performs the failing attach against a different session name ("omar-terminal-does-not-exist"), so it doesn't actually exercise the intended invariant about leaving the attempted session unchanged on failure. Consider rewriting it to make the attempted (missing) session a prefix of a real neighbour session, so the test would catch any accidental tmux prefix resolution causing the neighbour's mouse option to be flipped.

        // A name that does not resolve: the attach fails before any viewer.
        assert!(Attachment::open("", "omar-terminal-does-not-exist").is_err());

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.