8000 fix(minimal): fix race when uploading to worktree by twitchyliquid64 · Pull Request #806 · gominimal/minimal · GitHub
[go: up one dir, main page]
Skip to content

fix(minimal): fix race when uploading to worktree #806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
twitchyliquid64 merged 1 commit into from
Jul 17, 2026
Merged

fix(minimal): fix race when uploading to worktree #806

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 16 additions & 5 deletions crates/minimal/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl Client {
let subsystem =
constcat::concat!(minimald_rpc::RPC_SUBSYSTEM_PREFIX, "WorkspaceFilesTarZst");

let channel = self
let mut channel = self
.handle
.channel_open_session()
.await
Expand All @@ -217,11 +217,22 @@ impl Client {
.await
.context("request WorkspaceFilesTarZst subsystem")?;

let stream = channel.into_stream();
crate::file_upload::stream_tar_zstd(dir, channel.make_writer()).await?;

// Stream a tar+zstd archive directly to the daemon channel,
// avoiding buffering the entire archive in memory.
crate::file_upload::stream_tar_zstd(dir, stream).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);
}
}
Comment on lines +222 to +228
@coderabbitai coderabbitai Bot Jul 17, 2026
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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant file around the cited lines
sed -n '180,260p' crates/minimal/src/client.rs | cat -n

# Find timeout-related policy references in the repo
rg -n "upload|ssh|timeout|deadline|Duration::from|tokio::time::timeout" crates -g '!target' -g '!node_modules'

# Find the channel wait / close pattern elsewhere
rg -n "channel\.wait\(\)\.await|ChannelMsg::ExtendedData|Wait for the channel to close" crates -g '!target' -g '!node_modules'

Repository: gominimal/minimal

Length of output: 50373

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for upload-specific timeout constants/policy in the minimal client and nearby modules
rg -n "UPLOAD_TIMEOUT|upload.*timeout|timeout.*upload|SSH.*timeout|ssh.*timeout|deadline.*upload|workspace file upload" crates/minimal crates/minimald crates -g '!target' -g '!node_modules'

# Inspect the top of the client file for constants and surrounding guidance
sed -n '1,140p' crates/minimal/src/client.rs | cat -n

# Inspect the upload caller to see whether it already wraps this in a timeout
sed -n '960,1010p' crates/minimal/src/lib.rs | cat -n

Repository: gominimal/minimal

Length of output: 8885

Bound the upload-completion wait. channel.wait().await can hang forever if unpacking stalls or the daemon never closes the channel; wrap this loop in a timeout and drop the channel on expiry.

🤖 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 222 - 228, Wrap the channel wait
loop around channel.wait().await in a timeout so stalled unpacking or an
unclosed daemon channel cannot block indefinitely. On timeout, drop the channel
before returning or propagating the timeout error, while preserving stderr
collection and normal completion behavior.

if !err.is_empty() {
anyhow::bail!(
"daemon failed to unpack project files: {}",
String::from_utf8_lossy(&err)
);
}
Ok(())
}
}

Expand Down
0