feat(minimald,minvmd): serve sessions directly over the vsock bridge - #374

crates/minimald/src/connection.rs (1)

148-153: ⚡ Quick win

Consider propagating the error instead of unwrap.

The .unwrap() at line 152 will panic if russh::server::run_stream fails during initialization. While this is spawned in a task (per server.rs), an explicit error type would allow the spawning code to log session setup failures rather than silently panic the task.

♻️ Proposed error propagation

Change the return type to propagate initialization errors:

     pub(crate) async fn from_stream<S>(
         s: S,
         c: Arc<RuConfig>,
         serv: ServerStateHandle,
         is_local: bool,
-    ) -> (ConnectionHandle, RunningSession<ConnectionHandler>)
+    ) -> Result<(ConnectionHandle, RunningSession<ConnectionHandler>), russh::Error>
     where
         S: AsyncRead + AsyncWrite + Unpin + Send + 'static,
     {
         let h = ConnectionHandle(Arc::new(Mutex::new(Self {
             auth: if is_local { Auth::Local } else { Auth::Pending },
             ssh_username: None,
             channels: BTreeMap::new(),
             serv,
         })));
 
-        (
+        Ok((
             h.clone(),
-            russh::server::run_stream(c, s, ConnectionHandler(h))
-                .await
-                .unwrap(),
-        )
+            russh::server::run_stream(c, s, ConnectionHandler(h)).await?,
+        ))
     }

Then update call sites in server.rs to handle the Result.

🤖 Prompt for 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.

In `@crates/minimald/src/connection.rs` around lines 148 - 153, The code currently
calls russh::server::run_stream(c, s, ConnectionHandler(h)).await.unwrap(),
which will panic on initialization failure; change this to propagate the error
instead of unwrapping: have the enclosing function return a Result and replace
.unwrap() with ? (or map_err to your crate error type) so the run_stream error
is returned, and update the caller(s) in server.rs to handle/log the Result
rather than assuming success; keep references to h.clone(),
russh::server::run_stream, and ConnectionHandler(h) when locating where to
change the return type and error propagation.

Fix all unresolved CodeRabbit comments on this PR:

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

crates/minimald/src/main.rs (1)

13-14: ⚡ Quick win

Misleading "guest relay" terminology.

The doc comment refers to "guest relay," but this PR removes the in-guest socat relay in favor of minimald serving directly on vsock. Consider replacing "guest relay" with "guest server" or "minimald vsock listener" to avoid confusion with the relay architecture that is being removed.

📝 Suggested doc comment clarification

-/// Default AF_VSOCK port the guest relay listens on (the boot-contract bridge
+/// Default AF_VSOCK port minimald listens on in the guest (the boot-contract bridge
 /// port the host registers via `krun_add_vsock_port2`).

🤖 Prompt for 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.

In `@crates/minimald/src/main.rs` around lines 13 - 14, Update the doc comment
that currently calls it the "guest relay" to avoid confusion with the removed
in-guest socat relay—change phrasing above the default AF_VSOCK port constant
(the comment describing the default AF_VSOCK port the guest relay listens on) to
something like "guest server" or "minimald vsock listener" so it accurately
describes minimald serving directly on vsock.

.github/workflows/ci-macos.yml (2)

179-186: ⚡ Quick win

Missing boot log capture for session E2E debugging.

The boot_e2e step sets MINVMD_BOOT_LOG (line 171) and autospawn-e2e does too (line 266), but this step omits it. If the session E2E fails, there's no guest console output to diagnose whether the issue is boot-related or session-related.

Consider adding MINVMD_BOOT_LOG="$RUNNER_TEMP/session-boot.log" for consistency and debuggability.

🤖 Prompt for 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.

In @.github/workflows/ci-macos.yml around lines 179 - 186, Add setting
MINVMD_BOOT_LOG for the session E2E run so the guest console is captured for
debugging; in the run block where testbin (resolved from
target/debug/deps/minimald_session_e2e-*) is executed with MINVMD_E2E and
MINVMD_KERNEL_PATH/MINVMD_ROOTFS_PATH/MINVMD_INITRAMFS, also export
MINVMD_BOOT_LOG="$RUNNER_TEMP/session-boot.log" before invoking the test binary
(the one that runs minimald_session_e2e and the minimald_exec_over_bridge test)
to match boot_e2e and autospawn-e2e behavior.

---

267-272: 💤 Low value

Temp directory for XDG_RUNTIME_DIR is never cleaned up.

The temp directory created at line 267 is not removed on exit. The EXIT trap only stops minvmd. On a persistent self-hosted runner, this leaks a temp directory per run.

Consider adding cleanup to the trap:

🧹 Suggested fix

           export XDG_RUNTIME_DIR="$(mktemp -d)"
+          _xdg_cleanup() { rm -rf "$XDG_RUNTIME_DIR" 2>/dev/null || true; }
           # ...
-          trap 'minvmd stop >/dev/null 2>&1 || true' EXIT
+          trap 'minvmd stop >/dev/null 2>&1 || true; _xdg_cleanup' EXIT

🤖 Prompt for 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.

In @.github/workflows/ci-macos.yml around lines 267 - 272, The temp dir created
for XDG_RUNTIME_DIR is never removed; store the result of mktemp -d in a
variable (e.g., TMP_XDG_RUNTIME_DIR), export
XDG_RUNTIME_DIR="$TMP_XDG_RUNTIME_DIR", and extend the existing EXIT trap that
currently stops minvmd to also remove that temp directory (rm -rf
"$TMP_XDG_RUNTIME_DIR") so the temp dir is cleaned up on script exit; ensure the
trap covers all exit signals as before and that the variable name matches where
XDG_RUNTIME_DIR is referenced.

Fix all unresolved CodeRabbit comments on this PR:

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