8000 feat(minimald): RPC to stream files to session homedir by twitchyliquid64 · Pull Request #862 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions crates/minimal/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,53 @@ impl Client {
}
Ok(())
}

/// Stream a zstd-compressed tarball of `dir` to the daemon's
/// `HomedirFilesTarZst` subsystem, which unpacks it into the
/// session's home directory.
///
/// `session_id` is set as the `MINIMAL_SESSION_ID` env var on the
/// channel so the daemon can scope the upload to the correct session.
pub async fn upload_home_files(
&mut self,
session_id: sessions::SessionId,
dir: &Path,
) -> Result<(), anyhow::Error> {
let subsystem = constcat::concat!(minimald_rpc::RPC_SUBSYSTEM_PREFIX, "HomedirFilesTarZst");

let mut channel = self
.handle
.channel_open_session()
.await
.context("open channel for workspace file upload")?;
Comment on lines +251 to +255
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the upload error context.

Line 255 reports a workspace upload when this method uploads homedir files, which misleads users during channel-open failures.

Proposed fix
-            .context("open channel for workspace file upload")?;
+            .context("open channel for homedir file upload")?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let mut channel = self
.handle
.channel_open_session()
.await
.context("open channel for workspace file upload")?;
let mut channel = self
.handle
.channel_open_session()
.await
.context("open channel for homedir file upload")?;
🤖 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/minimal/src/client.rs` around lines 251 - 255, Update the error
context on the channel_open_session call in the homedir file upload method to
identify homedir file upload rather than workspace file upload. Preserve the
existing await and error propagation behavior.


channel
.set_env(true, "MINIMAL_SESSION_ID", session_id.to_string())
.await
.context("set MINIMAL_SESSION_ID env")?;

channel
.request_subsystem(true, subsystem)
.await
.context("request HomedirFilesTarZst subsystem")?;

crate::file_upload::stream_tar_zstd(dir, channel.make_writer()).await?;

// Wait for the channel to close to signal that unpacking is done.
let mut err = Vec::new();
while let Some(msg) = channel.wait().await {
if let russh::ChannelMsg::ExtendedData { data, ext: 1 } = msg {
err.extend_from_slice(&data);
}
}
if !err.is_empty() {
anyhow::bail!(
"daemon failed to unpack homedir files: {}",
String::from_utf8_lossy(&err)
);
}
Ok(())
}
}

/// Resolve the provider-instance dir (`<state dir>/providers/local-0`) the
Expand Down
85 changes: 81 additions & 4 deletions crates/minimald/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,21 +524,41 @@ async fn serve_stream_workspace_files(
config: ChannelConfig,
mut c: RuChannel<Msg>,
) {
if let Err(msg) = unpack_workspace_files(&s, &config, &mut c).await {
if let Err(msg) = unpack_files(&s, &config, &mut c, UnpackDir::Workspace).await {
let _ = c.extended_data_bytes(1, msg).await;
}
let _ = c.close().await;
}

pub(crate) const STREAM_HOMEDIR_FILES: &str =
constcat::concat!(RPC_SUBSYSTEM_PREFIX, "HomedirFilesTarZst");

async fn serve_stream_homedir_files(
s: ServerStateHandle,
config: ChannelConfig,
mut c: RuChannel<Msg>,
) {
if let Err(msg) = unpack_files(&s, &config, &mut c, UnpackDir::Homedir).await {
let _ = c.extended_data_bytes(1, msg).await;
}
let _ = c.close().await;
}

enum UnpackDir {
Workspace,
Homedir,
}

/// Unpacks the zstd-compressed tarball streamed over `c` into the
/// workspace directory of the session named by the channel environment.
/// given directory of the session named by the channel environment.
///
/// On failure, returns the human-readable message to relay back to the
/// client over the channel's extended-data stream.
async fn unpack_workspace_files(
async fn unpack_files(
s: &ServerStateHandle,
config: &ChannelConfig,
c: &mut RuChannel<Msg>,
into_dir: UnpackDir,
) -> Result<(), String> {
let session_id_str = config
.env_vars
Expand All @@ -562,7 +582,13 @@ async fn unpack_workspace_files(
c.make_reader(),
));
async_tar::Archive::new(reader)
.unpack(paths.working.as_utf8_path())
.unpack(
match into_dir {
UnpackDir::Workspace => paths.working,
UnpackDir::Homedir => paths.home,
}
.as_utf8_path(),
)
.await
.map_err(|e| format!("unpack failed: {e}"))?;

Expand Down Expand Up @@ -603,6 +629,7 @@ pub async fn handle_ssh_rpc(
| GetSessionPolicy::NAME
| GetMeshStatus::NAME
| STREAM_WORKSPACE_FILES
| STREAM_HOMEDIR_FILES
| IssueClientCert::NAME => {
let mut conn_lock = c.lock().await;
let c_hnd = match conn_lock.take(id) {
Expand Down Expand Up @@ -642,6 +669,7 @@ pub async fn handle_ssh_rpc(
GetSessionPolicy::NAME => drop(spawn(serve_get_session_policy(s, channel))),
GetMeshStatus::NAME => drop(spawn(serve_get_mesh_status(s, channel))),
STREAM_WORKSPACE_FILES => drop(spawn(serve_stream_workspace_files(s, config, channel))),
STREAM_HOMEDIR_FILES => drop(spawn(serve_stream_homedir_files(s, config, channel))),
IssueClientCert::NAME => {
#[cfg(feature = "networking-proxy")]
drop(spawn(serve_issue_client_cert(s, channel)));
Expand Down Expand Up @@ -784,6 +812,55 @@ mod tests {
);
}

#[tokio::test]
async fn stream_homedir_files_unpacks_tarball_into_home() {
let server = TestServer::new().await;
let mut client = server.connect().await;
let session_id = fresh_session(&mut client).await;

let payload = tar_zst(&[
("hello.txt", b"hello world\n"),
("dir/nested.txt", b"nested contents"),
])
.await;

let channel = client
.open_subsystem(
STREAM_HOMEDIR_FILES,
&[(MINIMAL_SESSION_ID_ENV, &session_id.to_string())],
)
.await;
let mut stream = channel.into_stream();
stream.write_all(&payload).await.unwrap();
// Half-close so the server's decoder sees EOF; then read to the
// server's channel close so the unpack has completed on-disk
// before we assert.
stream.shutdown().await.unwrap();
let mut trailing = Vec::new();
stream.read_to_end(&mut trailing).await.unwrap();

let mngr = server.state.sessions_manager().await;
let handle = mngr
.get_session(SessionKeyPredicate::Id(session_id))
.await
.unwrap()
.expect("freshly-created session should be retrievable");
let paths = handle.paths().await.unwrap();

assert_eq!(
tokio::fs::read(paths.home.as_utf8_path().join("hello.txt"))
.await
.unwrap(),
b"hello world\n",
);
assert_eq!(
tokio::fs::read(paths.home.as_utf8_path().join("dir/nested.txt"))
.await
.unwrap(),
b"nested contents",
);
}

#[tokio::test]
async fn get_version_returns_compiled_in_versions() {
let server = TestServer::new().await;
Expand Down
0