feat(minimald): implement scaffolding for execution over ssh by twitchyliquid64 · Pull Request #281 · gominimal/minimal

twitchyliquid64 · 2026-06-02T00:26:15Z

The main piece is the new ExecTask struct, which represents and async task & encapsulates the logic gluing a process to an ssh channel. ExecTask is generic over a Spawnable trait, which is something that can build a process (in the future, this can also be sandboxed processes ala minimal tasks).

For now I've just got it running unsandboxed processes as a simple way to test - we won't be doing this long term.

I'm also thinking in the long term ExecTask can probably be an actor, and recieve messages to do things like kill its process etc. And we could have the closure in spawn() register the ExecTask with a session and/or connection.

Summary by CodeRabbit

New Features
- Implemented SSH exec request handling: remote command execution with stdin/stdout/stderr streaming, working-directory and environment support, and exit status reporting.
Tests
- Added unit and end-to-end tests covering stdio bridging, exit-code behavior, error handling, and a test harness helper to exercise exec requests in tests.

coderabbitai · 2026-06-02T00:26:25Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: de3962eb-db7d-4884-8b47-4a2854f17958

📥 Commits

Reviewing files that changed from the base of the PR and between 101342e and 1a0fd52.

📒 Files selected for processing (5)

crates/minimald/src/connection.rs
crates/minimald/src/exec.rs
crates/minimald/src/lib.rs
crates/minimald/src/sftp.rs
crates/minimald/src/test_harness.rs

✅ Files skipped from review due to trivial changes (1)

crates/minimald/src/lib.rs

🚧 Files skipped from review as they are similar to previous changes (4)

crates/minimald/src/connection.rs
crates/minimald/src/exec.rs
crates/minimald/src/sftp.rs
crates/minimald/src/test_harness.rs

📝 Walkthrough

Walkthrough

Adds a new exec module implementing async subprocess spawn/bridge, validates and dispatches SSH exec requests (using MINIMAL_SESSION_ID_ENV), wires exec handling into the connection code, centralizes the env constant, and adds unit and end-to-end tests plus a TestClient::exec helper.

Changes

SSH Exec Request Implementation

Layer / File(s)	Summary
Shared environment variable contract `crates/minimald/src/lib.rs`, `crates/minimald/src/sftp.rs`	`MINIMAL_SESSION_ID_ENV` constant is centralized in lib.rs and imported by sftp.rs; related crate imports were consolidated.
Process abstraction traits and Tokio implementations `crates/minimald/src/exec.rs`	Introduces `Spawnable` and `Process` async traits and production implementations `TokioSpawn`/`TokioProcess` that execute `/bin/sh -c` with cwd/env and piped stdio.
Exec task and stdio bridging `crates/minimald/src/exec.rs`	`ExecTask::run` wires SSH channel readers/writers to a spawned process; `bridge` concurrently forwards stdin→child stdin, child stdout→SSH, child stderr→SSH extended, and kills the child on SSH write failures.
Exec request validation and dispatch `crates/minimald/src/exec.rs`	`handle_exec` decodes argv UTF‑8, rejects PTY requests, requires and parses `MINIMAL_SESSION_ID_ENV` as a session ID, looks up session state, marks channel success, and spawns an async ExecTask with `TokioSpawn`.
Connection handler integration `crates/minimald/src/connection.rs`	Adds `mod exec` import, derives `Debug` for `ConnectionHandle`, and changes `exec_request` to call `exec::handle_exec(...).await`.
Test infrastructure and validation `crates/minimald/src/exec.rs`, `crates/minimald/src/test_harness.rs`	Adds mock `Process` and duplex endpoints for unit tests (bridge and failure modes), end-to-end Tests using `TestServer`, and `TestClient::exec` with `ExecOutcome` and `ExecRejected`.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 I tunneled through channels to spawn a new thread,
Pipes humming softly where bytes boldly tread,
Stdin and out danced across SSH night,
Tests kept the bridges held firm and tight,
A rabbit applauds the exec now fed.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: implementing SSH exec request handling scaffolding. It is concise, specific, and directly reflects the core contribution of adding exec task logic, trait abstractions, and related infrastructure.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/minimald/src/exec.rs`:
- Around line 167-173: The code currently calls unwrap() on w.flush().await,
e.flush().await, ws.eof().await, ws.exit_status(...).await and ws.close().await
after drop(r), which will panic if the SSH channel is broken; change these to
handle errors gracefully instead of panicking — for each call in exec.rs replace
the unwraps with non-panicking handling (e.g., match/if let Err(err) = ... {
log/warn the error with context using the task logger or tracing, and ignore or
recover as appropriate) or use let _ = ... to explicitly discard errors where
safe; ensure you keep drop(r) and still attempt eof/exit_status/close but
tolerate and log failures for ws.eof, ws.exit_status, ws.close and for
w.flush/e.flush so the task does not crash on broken channels.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 37dd2c51-243c-4c80-ab26-4ea7def8a9a9

📥 Commits

Reviewing files that changed from the base of the PR and between 48eda35 and 101342e.

📒 Files selected for processing (5)

crates/minimald/src/connection.rs
crates/minimald/src/exec.rs
crates/minimald/src/lib.rs
crates/minimald/src/sftp.rs
crates/minimald/src/test_harness.rs

evanspearman · 2026-06-02T14:43:51Z

+    /// Resolves when the process exits. `Ok(None)` means the process
+    /// was terminated by a signal rather than exiting with a numeric
+    /// code.
+    async fn wait(&mut self) -> io::Result<Option<i32>>;


Would there be any reason we might want to capture the specific signal?

Yes, on one side we need to communicate the exit status down the ssh channel, but also, every process needs to be wait()’ed on to avoid zombie processes.

twitchyliquid64 requested a review from evanspearman June 2, 2026 00:26

coderabbitai Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread crates/minimald/src/exec.rs Outdated

feat(minimald): implement scaffolding for execution over ssh

1a0fd52

twitchyliquid64 force-pushed the tom/update branch from 101342e to 1a0fd52 Compare June 2, 2026 00:34

twitchyliquid64 requested a review from 0chroma June 2, 2026 00:36

evanspearman approved these changes Jun 2, 2026

View reviewed changes

twitchyliquid64 merged commit 5628477 into main Jun 2, 2026
9 checks passed

twitchyliquid64 deleted the tom/update branch June 2, 2026 18:36

This was referenced Jun 4, 2026

feat(minimald): initial, shitty impl of task execution #338

Merged

feat(minimald): handle git pushes natively #347

Merged

This was referenced Jun 11, 2026

feat(minimald): scaffolding for the attachable session process #375

Merged

fix(minimald): plumb errors for spawning/attaching to session shells #383

Merged

coderabbitai Bot mentioned this pull request Jul 10, 2026

feat(minimald): rip out arbitrary session exec #709

Merged

Navigation Menu

Uh oh!

feat(minimald): implement scaffolding for execution over ssh - #281

feat(minimald): implement scaffolding for execution over ssh#281
twitchyliquid64 merged 1 commit into
mainfrom
tom/update

twitchyliquid64 commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

evanspearman Jun 2, 2026

Uh oh!

twitchyliquid64 Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

twitchyliquid64 commented Jun 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evanspearman Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

twitchyliquid64 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twitchyliquid64 commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading