fix(files): resolve diff-accept SaveFuture on real write outcome - #74
Merged
Conversation
`TuiDiffStorage::dispatch_write` resolved its `SaveFuture` on successful
*dispatch* (an already-ready `Ok`), because `FileModel`'s save/delete/
rename_and_save are event-based: they spawn the write and report completion
later via `FileModelEvent::{FileSaved, FailedToSave}`. So `accept_and_save`
reported `Success` even when the async write failed — a silent divergence
from the accept contract `DiffStorageHelper` assumes (it joins the futures
and maps any error to `DiffApplicationFailed`).
Add a completion-waiter mechanism to `FileModel`:
- a `save_waiters: HashMap<FileId, Vec<oneshot::Sender<..>>>` field,
- a shared `report_save_outcome` funnel that every write-completion callback
(local/remote save, rename, local/remote delete) now routes through — it
performs the exact same `set_version` + `FileSaved`/`FailedToSave` emit as
before, then additionally resolves any waiters for the file,
- `save_completion(file_id)` returning a future that resolves `Ok(())` on
`FileSaved` / the error on `FailedToSave`.
`dispatch_write` now registers a `save_completion` waiter for a successful
dispatch and returns that future, so a failed write (e.g. a create whose
parent path is a file, not a directory) surfaces as `DiffApplicationFailed`.
The waiter is registered synchronously before the spawned write can resolve;
a synchronous dispatch error still short-circuits without leaving a waiter.
Fixes `accept_reports_write_dispatch_failure` (now reports the failure) and
`accept_renames_and_reports_source_as_deleted` (the completion future removes
the "write not drained by .await" flakiness). warp_files 30/0, all 9
tui_diff_storage tests green, `warp` builds clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gDYSHa4oDvQbfungWwG1h
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #4. Fixes 2 of the remaining serial failures (the "diff-storage" pair).
Problem
TuiDiffStorage::dispatch_writeresolved itsSaveFutureon successful dispatch, becauseFileModelis event-based —save/delete/rename_and_savespawn the write and report completion later viaFileModelEvent::{FileSaved, FailedToSave}. Soaccept_and_savereportedSuccesseven when the actual write failed, diverging from the accept contract (DiffStorageHelperjoins the save futures and maps any error toDiffApplicationFailed).Fix
Add a completion-waiter to
FileModel:save_waiters: HashMap<FileId, Vec<oneshot::Sender<..>>>report_save_outcomefunnel every write-completion callback routes through — identicalset_version+FileSaved/FailedToSavebehavior as before, plus resolving any waiterssave_completion(file_id)→ a future resolvingOk(())onFileSaved/ the error onFailedToSavedispatch_writeregisters a waiter for a successful dispatch and returns that future. The waiter is registered synchronously before the spawned write can resolve; a synchronous dispatch error still short-circuits.Result
accept_reports_write_dispatch_failure— now reportsDiffApplicationFailedwhen the parent-dir creation fails.accept_renames_and_reports_source_as_deleted— the completion future removes the "write not drained by.await" flake.tui_diff_storagetests green;warpbuilds clean.No existing event behavior changed; no assertions weakened.
🤖 Generated with Claude Code