fix(sandbox): track PTY state per SSH channel to fix terminal resize

[cluster2600]

Summary

Resolves #543 — terminal resize (window_change_request) was broken when multiple SSH channels were open simultaneously.

• Replace the flat pty_master/input_sender/pty_request fields in SshHandler with a HashMap<ChannelId, ChannelState>, ensuring each channel tracks its own PTY resources independently
• Fix set_winsize to pass a reference (&winsize) to ioctl, correcting undefined behaviour on aarch64
• Add error logging when set_winsize fails, rather than silently discarding the result

Root Cause

The previous implementation stored a single pty_master file descriptor at the handler level. When a client opened multiple channels (e.g. parallel shells, shell + SFTP), window_change_request would resize whichever PTY was stored last — not the one belonging to the requesting channel.

graph TD
    subgraph "Before (broken)"
        A[Channel 0 — resize 80×24] --> B[SshHandler.pty_master]
        C[Channel 1 — resize 120×50] --> B
        B --> D["ioctl(TIOCSWINSZ) on last-stored FD"]
    end

    subgraph "After (fixed)"
        E[Channel 0 — resize 80×24] --> F["channels[0].pty_master"]
        G[Channel 1 — resize 120×50] --> H["channels[1].pty_master"]
        F --> I["ioctl(TIOCSWINSZ) on channel 0 FD"]
        H --> J["ioctl(TIOCSWINSZ) on channel 1 FD"]
    end

Loading

Changes

File	Change
crates/openshell-sandbox/src/ssh.rs	Introduce ChannelState struct; migrate all per-channel fields into HashMap<ChannelId, ChannelState>; update pty_request_request, window_change_request, data, channel_eof, subsystem_request, and spawn_shell_or_exec to look up channel state by ID

Test Plan

• cargo test -p openshell-sandbox — 319 passed (1 pre-existing failure in drop_privileges, unrelated)
• New test: set_winsize_applies_to_correct_pty — verifies two PTYs can be resized independently
• New test: channel_state_independent_input_senders — verifies data routing and EOF isolation between channels

sequenceDiagram
    participant Client
    participant SshHandler
    participant Channel0
    participant Channel1

    Client->>SshHandler: pty_request(channel=0, 80×24)
    SshHandler->>Channel0: store PTY master + request

    Client->>SshHandler: pty_request(channel=1, 120×50)
    SshHandler->>Channel1: store PTY master + request

    Client->>SshHandler: window_change(channel=0, 132×43)
    SshHandler->>Channel0: ioctl(TIOCSWINSZ, 132×43)
    Note over Channel1: unaffected

    Client->>SshHandler: channel_eof(channel=0)
    SshHandler->>Channel0: drop input_sender
    Note over Channel1: still functional

Loading

[github-actions]

Thank you for your interest in contributing to OpenShell, @cluster2600.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

1. Open a Vouch Request discussion.
2. Describe what you want to change and why.
3. Write in your own words — do not have an AI generate the request.
4. A maintainer will comment /vouch if approved.
5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[cluster2600]

I have read the DCO document and I hereby sign the DCO.

[johntmyers]

Review: fix(sandbox): track PTY state per SSH channel to fix terminal resize

The bug is real, the fix is architecturally sound, and the scope is tight. Two items to address before merge, plus some suggestions.

Blocking

ChannelState entries are never cleaned up — FD leak

There's no channel_close handler, so self.channels grows monotonically. Each ChannelState holds an Option<std::fs::File> (the PTY master fd). For long-lived connections (VS Code Remote-SSH, agent sessions that open many channels), this will exhaust file descriptors.

channel_eof only drops the input_sender — that's correct for stdin EOF semantics, but channel_close is where full cleanup belongs (EOF and close are distinct in SSH protocol).

Suggested fix:

async fn channel_close(
    &mut self,
    channel: ChannelId,
    _session: &mut Session,
) -> Result<(), Self::Error> {
    self.channels.remove(&channel);
    Ok(())
}

Suggestions

Derive Default on ChannelState

The or_insert_with(|| ChannelState { input_sender: None, pty_master: None, pty_request: None }) pattern is repeated 3 times. #[derive(Default)] + .or_default() would reduce the repetition.

Initialize state in channel_open_session instead of lazily

Inserting the entry at channel open and removing it at channel close makes the lifecycle explicit. Then pty_request/start_shell/subsystem_request can use get_mut instead of entry().or_insert_with(), which would surface bugs where a channel is used without being opened rather than silently creating an empty entry.

Test coverage note

set_winsize_applies_to_correct_pty validates that two kernel PTYs have independent sizes, and channel_state_independent_input_senders validates mpsc::Sender isolation — both are really testing OS/stdlib guarantees rather than the channel-routing logic. They still have documentary value, but a test that populates two ChannelState entries in an SshHandler and verifies window_change_request dispatches to the correct one would directly validate the fix. Understand this may be hard given SshHandler's dependencies — fine to note the limitation and skip.

Looks good

• Core change (flat fields -> HashMap<ChannelId, ChannelState>) is the right fix, well-documented.
• The &winsize ioctl fix is correct — passing by value to a variadic function was wrong on all architectures.
• Adding warn! on set_winsize failure is a good improvement over silent discard.
• Scope is clean — three related fixes, no unrelated changes.