Repository navigation
File browser: a running download no longer blocks other SFTP operations of the same session; file I/O off the async workers #921
Copy link
Copy link
Open
Description
Goal
While a large file downloads, the user can keep browsing the same server, and AI file reads in that session do not wait for the download to finish.
Context
crates/app-shell/src/commands/sftp/transfer.rs:124-200download_file:lock_browser_sftp(run.session, run.channel)(:150, defined incrates/app-shell/src/commands/elevation.rs:690) is held for the wholeread_file_chunkedloop. Every other SFTP operation of the session – listing, preview, AI file reads (crates/app-logic/src/orchestration/remote_files.rs) – waits until the transfer ends.- Inside the chunk callback the local file is written with blocking
std::fs(file.write_all,:157) on a runtime worker;std::fs::rename(:183) andremove_file(:197) as well. Uploads in the same module andcommands/sftp/folder_upload.rsfollow the same pattern – check them. - The normal SFTP channel and the transport of a session form a pair (Spec 0086 A3); the elevated channel has its own slot and revocation rules (Specs 0085, 0086;
revoked_mid_command). - Transfers have progress, stop and revocation handling (Issue File browser: chunked transfers with progress and cancel; ending elevated rights asks before stopping running elevated operations #622) that must keep working.
Requirements
- Long transfers do not hold the session's SFTP lock across the whole transfer. Recommended: take the browser SFTP guard per chunk (read one chunk, release, write it locally, repeat), with the stop/revocation check between chunks. Alternative for the normal channel: a dedicated SFTP channel per transfer opened on the same transport (keeps the A3 pairing because it is derived from the session's own transport); the elevated channel always uses the per-chunk approach.
- Local file I/O of transfers runs in
spawn_blocking(ortokio::fs), never as blockingstd::fson a runtime worker. - Elevated mode: revocation mid-transfer still ends the transfer with the revoked error (
run.channel.revoked_error()), and no chunk is read after revocation. - Apply the same to uploads if they show the same pattern.
Acceptance Criteria
- Test (mock SFTP with a per-chunk delay controlled by the test): during a download of 100 MB (simulated), a
sftp_listof the same session completes while the download is still running (ordering-based; upper bound 1 s). - Test: an AI remote file read during a running download completes before the download ends.
- Test: stopping a download mid-way leaves no temp file; revoking elevated mode mid-way ends with the revoked error and reads no further chunk.
- No
std::fs::call intransfer.rs/folder_upload.rsoutsidespawn_blockingclosures (guard test or review checklist in the PR). - Existing transfer, progress, stop and elevated-revocation tests (Issues File browser: chunked transfers with progress and cancel; ending elevated rights asks before stopping running elevated operations #622, File browser: transfer row stays at "Downloading" when elevated rights are switched off during the transfer #621, Specs 0085/0086) pass unchanged.
- Relevant automated tests pass.
Technical Notes
- Security-critical area: elevated SFTP channel and revocation (
.claude/skills/smart-ssh-coder/references/security-invariants.md). Releasing the lock between chunks must not allow a chunk to be read with a channel that was revoked or swapped in between – re-check the channel identity (expected_user) on every re-acquire. - Keep the lock order documented in
elevated_sftp.rs:414-420(SFTP before transport).
Spec impact: update 0086 (transfers no longer serialise the session's SFTP channel)
Open questions
- Per-chunk locking vs. dedicated channel for the normal channel: recommendation per-chunk (smaller change, same code for both channels); choose the dedicated channel only if per-chunk throughput drops by more than 20 % in the mock benchmark.
Related Work
- Blocked by app-shell sftp: split commands/sftp.rs into one file per command group #844 (split of
commands/sftp.rs) and File browser backend: move recursive delete/chmod/chown, mode-bit and owner logic from app-shell to app-logic::sftp_ops #849 (file operations moved toapp-logic::sftp_ops) to avoid conflicts. - File browser: chunked transfers with progress and cancel; ending elevated rights asks before stopping running elevated operations #622, File browser: transfer row stays at "Downloading" when elevated rights are switched off during the transfer #621, Keep the file browser, terminal and disconnect usable while a long command runs #328, Spec 0085, Spec 0086
Metadata
Metadata
Assignees
Type
Fields
Priority
P2 - Normal
Activity
3 remaining items