Repository navigation
fix: startup resilience, precise render window, file-transfer ownership safety, fail-closed migration - #370
Merged
Conversation
Tianyu199509
force-pushed
the
fix/startup-resilience-and-file-safety
branch
from
September 15, 2026 17:09
ce9bd0f to
604ec5f
Compare
…atomically EnsureItemRendered used to expand the render window to the whole folder whenever the target sat outside the rendered prefix; it now grows just far enough (chunk-aligned, index-bounded) with pure-function behavior tests. Rename/reveal paths cover window-external items and expand stacks first. Storage migration persists through SaveCheckedAsync (a silent disk failure no longer strands config and files on different roots), post-commit helpers never roll back, and the display watcher keeps a 30s safety poll instead of stopping it entirely once event-driven.
Copy rollback now deletes by receipt (source-twin length/timestamp match, bottom-up empty dirs only) instead of recursing into whatever lives at the destination - a user cancel can no longer take out files someone else placed there. Cross-volume moves capture NTFS file-key identity before the copy and revalidate before deleting the source; a changed source keeps both copies. The CreateNew-based safe copy core is now the only file-copy path, partial-destination cleanup verifies expected length, migration merge-back keeps diverged duplicates, and the drag attribute strip/restore pair is serialized and logged.
A failed migration step used to let the rest of the chain run anyway and the final schema version was stamped unconditionally, permanently skipping the failed step. The chain now advances strictly one step at a time, stops at the first failure, and records the checkpoint it reached so the failed step retries next launch (fault-injection tested per step). Update manifests must now come from pinned HTTPS hosts (deskbox.fun / github.com / objects.githubusercontent.com) for both x64 and arm64 installer URLs.
The aggregated HRESULT stays 0 while an individual phase fails, which hid where the Explorer delegation died (feedback #9). Failure details now include per-phase HRESULTs, and the open-busy message points at an unresponsive shell with the restart-explorer remedy.
Two 2026-09-15 reports: the six-track startup-resilience audit (zombie single-instance lock incident, its fix inventory, and what stays open) and the red-team DD adoption cross-check (verified claims, adjusted remediation rounds, decision items).
Source identity now requires the conjunction of volume serial, file key, length, and write time: a matching NTFS file key proves the same file object, not unchanged content, so an in-place edit after the copy must still block the source delete. An uncapturable identity (file systems that cannot stat open files) throws the source-cleanup exception instead of silently authorizing the delete - both copies stay in every unprovable case. The updater origin pin no longer trusts the whole github.com domain (a tampered manifest could point at any repository with a matching hash); only this project's own release download paths are accepted, and objects.githubusercontent.com is rejected as a manifest origin since the client follows redirects from the trusted release URL anyway. The startup watchdog now measures time since the last startup progress mark instead of total elapsed time: legitimately long phases (a multi- gigabyte pre-restore archive, per-widget restoration) keep marking progress and cannot be killed, while a real hang still trips the stall window. Also fixes the cross-thread pending-launch flag read.
Source deletion is now handle-bound: the path is opened once with delete access, the volume/file-key/length/timestamp identity is read from that same handle, and the POSIX-style disposition deletes exactly the verified object - closing the validate-to-delete race no amount of path re-checking could eliminate. An identity that cannot be captured keeps both copies. Destination ownership: the safe-copy core opens its destination via a native CreateNew handle with delete access, and a failed copy removes its partial file through that still-open handle instead of by path; rolling back a completed copy verifies a destination identity receipt first, so a replacement at the path is never cleaned up as ours. Directory rollbacks now use the child receipts recorded at copy time (actual "name (2)" destinations) instead of re-deriving destinations from the source tree, so a foreign file occupying the planned name can never be matched and deleted. Settings migration is atomic per step: a snapshot is taken before each step and restored verbatim when the step throws after mutating fields, so a failed migration leaves neither version progression nor half- applied state. File-key availability must be symmetric across stats or the identity check fails closed; asymmetric key visibility is treated as a replacement. The startup heartbeat now also marks progress inside the backup copy loop (every 32 MB), so a single multi-gigabyte file on slow storage cannot trip the stall watchdog.
The cross-volume file move now opens ONE source handle (read + delete, shared for reading only) that spans the copy through the final disposition: concurrent writers are refused up front instead of raced, and the deleted object is exactly the one that was copied - no gap remains anywhere between validation and deletion. The destination handle (CreateNew, write + delete, shared with nobody) stays open through the commit: metadata and the identity receipt are taken under that exclusive hold, a failed copy deletes its partial through the still-open handle, and kept-both-copies outcomes close it without deleting. The headless fallback now delegates to this same transaction instead of carrying a second copy-and-delete implementation. All rollback deletions converge on a single handle-bound primitive: the destination object is removed only through a freshly opened handle whose volume/file-key/length/timestamp identity matches the receipt captured at copy time (from the handle, before any close). Transfer records now carry that identity for plain files and for every directory child, so name-conflict destinations and completed-copy rollbacks can no longer be tricked into deleting a replacement that happens to match by path, length, and timestamp. The directory-tree source cleanup applies the same primitive per file, replacing the validate-the-whole-tree-then-delete-by-path window; files locked by other processes surface as cleanup failures (both copies kept) rather than being mistaken for replacements. Settings migration is copy-on-write: every step runs on a deserialized copy and the service swaps its settings reference to the returned graph, so a failed step leaves nothing behind - no reflection-based restore (and thus no NativeAOT metadata risk), no half-restored state reported as success. Win32 notes now encoded here: FILE_BASIC_INFO treats attributes == 0 as "no change" (a file whose only attribute was read-only must become FILE_ATTRIBUTE_NORMAL), and the delete check reads the read-only state snapshotted at handle open, so clearing it requires a fresh handle.
The generic identity delete now opens with read-only sharing, matching the move transaction's rule: a concurrent writer blocks the open and the file is kept (both copies) instead of being validated against one object and deleted against whatever replaced its content a moment later. Identity authority is all-or-nothing. Destinations without a recorded object identity are never deleted on rollback - the residue is logged for manual cleanup - and the directory-tree source cleanup fails closed as a cleanup exception instead of falling back to path deletes. File systems that provide no stable file keys cannot authorize destructive operations at all: metadata-only matches never authorize a delete (copies still work; the source simply stays, reported as "copied but could not safely finish the move"). Directory copies now capture the source identity from the very handle that performed the copy, never by re-statting the path afterwards - the old ordering recorded whatever had replaced the source by the time the copy finished, which could later authorize deleting that replacement while keeping the stale copy. A completed move records its destination object; a batch rollback moves a completed item back only while that object still occupies the destination path, so a replacement that appeared after the move is never swapped back. The move commit also closes the source handle before releasing the destination's exclusive hold, so one protected copy exists at every instant. Migration merge-back deletes duplicated copies through verified identity handles (the last path-based delete retired along with its helper).
…back
A cancel or mid-batch failure now keeps completed items, removes only the in-flight partial (through its own open handle), and never runs pending items. Rolling back completed work required deleting verified-owned objects whose source may have vanished meanwhile - the exact class of deletion this engine refuses to perform - and the reverse-move path had no safe answer for replacements appearing at a completed move's destination. Explorer semantics ('done is done') makes the entire category unreachable: the rollback engine, receipt-based entry rollback, reverse moves, and empty-destination-tree cleanup are deleted wholesale.
The object-identity deletions that remain are exactly two: the cross-volume move's handle-bound source disposition, and the generic verified-handle delete used by the directory-manifest source cleanup and migration merge-back. The migration merge-back additionally re-verifies the surviving original immediately before deleting its duplicate. The move transaction's source handle now also requests FILE_WRITE_ATTRIBUTES so read-only sources clear and delete through the same handle.
Desktop organization no longer reverse-moves completed items when a transaction fails: the recovery journal keeps every recorded receipt and the next launch's RecoverPendingAsync - the same verified path that already handles crashes (settings-commit checkpoint plus per-item snapshot matching) - restores what still matches. The immediate reverse move it replaces relocated whatever happened to sit at a destination path with no content verification at all. The managed-storage migration merge-back is now a conservative recursive merge: missing children move back, same-named directories merge child by child, and duplicates are never auto-deleted. The old path deleted a same-named copied subtree wholesale - after a partially failed source cleanup that subtree held the only remaining copy of every file already deleted from the original, an actual data-loss chain, and file-level deduplication trusted size and timestamp as an ownership proof. Leftover duplicates surface through the existing residue flow where the user decides. Both engines now attach CompletedResults to their cancellation and failure exceptions (wrapping the existing typed exceptions, whose base types keep every current catch compatible), so journals, history, and undo learn what physically finished under Explorer semantics; item- level receipts carried by inner exceptions (a move whose source cleanup failed after the copy completed) are merged into the wrapper.
File identities now use the full 128-bit id from GetFileInformationByHandleEx(FileIdInfo) instead of the 64-bit index (not unique on ReFS), captured through native handles opened with FILE_FLAG_BACKUP_SEMANTICS so directories identity the same way files do. Desktop organization receipts record the destination object identity right after each physical move completes (forward moves and undo candidates alike). Recovery and undo may only move an item while the object at the path still carries exactly that identity (volume + file id + the organization-time size/mtime as the conjunction baseline, so content edits after the move still fail the match). Items without a recorded identity - legacy journals, failed captures, or the rare crash between a physical move and its receipt - have no automatic restore authority: the file stays where it is and the journal keeps the record. The heuristic-only size+timestamp matching that let a replacement file be moved back is gone. The remaining partial-completion consumers catch up with Explorer semantics: MoveItemsBackToDesktop records physically completed items as an undoable history entry and only the rest as failures, and a failed plain undo marks each completed item as restored so a retry targets only what remains.
The FILE_ID_INFO interop declared the volume serial as 32-bit DWORD where the native struct uses ULONGLONG - the undersized 20-byte buffer made every GetFileInformationByHandleEx(FileIdInfo) call fail, so the 128-bit upgrade silently never ran and identity kept running on the legacy 64-bit view. The layout is now correct (24 bytes), the volume serial is ulong throughout, and the legacy 64-bit fallback is deleted: FileIdInfo is supported on every platform DeskBox runs on, so a failed query denies identity authority instead of degrading to the index that is not unique on ReFS. A plain-organization undo retry after a partial failure now plans only the items not yet restored - previously every item re-entered the plan, so the retry either aborted on the already-restored item's vanished target or relocated whatever reappeared at that path. The plan and its history item are paired explicitly, so completion bookkeeping cannot misalign after filtering. Undo receipts are durable in history: organization history items carry the destination identity captured at move time, and undo verifies against that recorded receipt instead of capturing a fresh identity at undo time - a replacement that later occupies the destination path no longer receives a valid id from DeskBox itself. Legacy history entries without an identity keep the pre-existing capture-on-undo behavior.
An identity failure before the cross-volume copy starts is now an ordinary IOException instead of a SourceCleanupException: nothing has been copied, so reporting completed results would pollute history, journals, and undo with a destination that does not exist. Every undoable move history entry now carries the destination identity captured when the physical move completed - managed drops and move-back-to-desktop join desktop organization, all through the same CaptureUndoReceiptIdentity helper. Plain-organization undo verifies the object at the destination against that recorded receipt before planning; a replacement that later occupies the path is rejected. Legacy history entries without a receipt no longer receive a freshly captured identity at undo time (that capture handed replacements a valid id); they keep their null receipt and get no automatic undo. The public-desktop stuck-reason check now passes the recorded identity through, so items read Busy instead of Changed again.
Tianyu199509
force-pushed
the
fix/startup-resilience-and-file-safety
branch
from
September 16, 2026 08:19
1f7f4be to
831505b
Compare
Every completed transfer result used to run its native open + double stat receipt capture on the UI thread's await chain, so a 2000-item drop froze the caller for seconds after the physical transfer had already finished. All four history-entry paths (managed drop and move-back-to-desktop, success and partial) now share one batch helper that captures receipts serially on a worker thread and reports the cost via [OrganizerPerf] receiptCount/receiptMs. Serial on purpose: the destination volume may be a slow SATA or USB device where concurrent metadata seeks regress - raise the parallelism only with measured evidence from the new logging.
The render window now guarantees a viewport-sized first page instead of relying on the extent-based fallback alone. A fixed 30-item prefix in a large widget viewport never overflows, so the ScrollViewer reports no extent, ViewChanged never fires, and unrendered files became unreachable with no scrollbar - files existed but looked gone. The view layer computes the minimum directly from its real viewport and item dimensions (grid icon cells; list row estimate) on Loaded, SizeChanged, icon-size changes, and scroll-viewer hookup; the view model clamps the request to the visible item count. The extent-based growth stays as a fallback and near-tail chunk growth keeps scrolling continuous. Copy imports (never undoable) no longer capture undo receipts at all, so a 2000-file copy drop skips the whole open+stat batch, not just the UI-thread cost.
The viewport fix in da445a6 still left three lifecycle dead ends where files existed but only the initial prefix rendered and nothing could scroll: - EnsureRenderWindowCoversViewport clamped the viewport page to the item count, so a check that raced ahead of a bulk import wrote 0 into the window. The ensure is now a pure raise of the budget. - ReconcileRenderWindowPrefix wrote a small folder's full-render count back into the window, so a later 2000-file import into the same widget stayed stuck at that stale prefix. The budget and the effective target are now separate state: the target is derived per reconcile (full below the activation threshold, budget-clamped above it) and nothing writes the rendered count back. - Bulk imports change the item count without any geometry change, so Loaded/SizeChanged never re-checked viewport coverage. The view model now raises RenderWindowSourceChanged from the coalesced reconcile callback and the view answers with a coalesced viewport check; ScrollViewer viewport dimensions are preferred over the items-control frame, and list rows measure the first realized container instead of assuming the icon cell height. Adds [RenderWindow] visible/rendered/budget logging above the threshold so a dead end is diagnosable from the log alone, behavior tests for 40->2088, 299->301 and the pre-load ensure, and source contract tests pinning the coverage loop wiring.
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
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.
Summary
Follow-up hardening batch after the 1.5.2 large-folder hotfix: startup resilience, precise render-window reveals, file-transfer safety by ownership/identity, fail-closed settings migration, pinned update origins, plus the explorer-launch observability fix from feedback #9. Full audit trail is in
docs/articles/startup-resilience-audit-20260915.mdanddocs/articles/redteam-dd-adoption-20260915.md.Startup resilience (6db2540)
A startup failure used to leave a headless process holding the single-instance mutex, which swallowed every later launch ("app won't open"). Startup is now split into a lifeline (dispatcher queue, settings load, tray, widget manager) and degradable feature steps; the fatal path always terminates — a dedicated-thread watchdog covers hangs, the report dialog has a forced-exit backstop and a re-entry gate. All
IsShownInSwitchers/SetPresentercall sites route through a degradingWindowShellStatehelper, teardown exits unconditionally via try/finally, and dropped startup tasks report throughSafeFireAndForget.Render window + storage commit boundary (f53b59c)
EnsureItemRenderedused to expand the render window to the whole folder whenever the target sat outside the rendered prefix — reintroducing the large-folder layout hang on reveal/rename paths. It now grows just far enough (chunk-aligned, index-bounded), with pure-function behavior tests (5000-item folders). Storage migration persists viaSaveCheckedAsyncso a silent disk failure rolls back instead of stranding config and files on different roots; the display watcher keeps a 30s safety poll instead of none.File-transfer safety (b64e30c)
CreateNew-based safe copy core is the only file-copy path; partial-destination cleanup verifies expected length; migration merge-back keeps diverged duplicates; the drag attribute strip/restore pair is serialized and logged.Fail-closed migration + update origins (1ffb244)
A failed settings-migration step used to let the rest of the chain run and the final schema version was stamped unconditionally, permanently skipping the failed step. The chain now advances strictly step-by-step, stops at the first failure, and records its checkpoint (fault-injection tested). Update manifests must come from pinned HTTPS hosts for both x64 and arm64 installer URLs.
Feedback #9 observability (79b153e)
Explorer-delegated launches now report per-phase HRESULTs (the aggregate stays 0 while a phase fails), and the open-busy message points at an unresponsive shell with the restart-explorer remedy. 12 languages updated.
Test plan
dotnet test -p:Platform=x64: 3586/3586 green (new: startup-resilience contracts, render-window behavior tests, source-identity/cancel-rollback behavior tests, migration fault injection, origin pinning)OnLaunched completed successfully, slow display poll confirmed in log, 90s watchdog no false positive)