Repository navigation
Commit e80d192
committed
fix(editor): release abandoned diff streams and owned placeholders (split 5/6 of #1066, unit U6a)
Diff-view half of #1932 replayed onto main tip 53cb5b5: discardUnapprovedStream
and discardOwnedPlaceholder cover streams that were abandoned, changed out from
under a retained session, or left by an unconfirmed save; isEditingDifferentPath
guards a second path while a session is retained, and placeholderPath stays owned
until the rollback removes it. The tool-side half moves to its own unit; plan and
measured widths in #1937 (comment).
The spec keeps its dirty-flag assignments legal after upstream rewrote the doubles
with the typed test-utils helpers: one locally widened ActiveDiffEditor type on the
shared builder instead of eight assertions, because vscode types TextDocument.isDirty
as read-only.
Review pass on the CodeRabbit change-request filed at this head: a refused buffer restore
now stops the rollback outright the same way an unconfirmed save of the restored buffer
does, so the tabs stay open, nothing is unlinked and the placeholder keeps its owner -- the
spec for that path now requires that no unlink happens instead of asserting one. The
commit-point test seeds placeholderPath so its release assertion can fail, and
isEditingDifferentPath / editingRelPath are pinned directly (retained path accepted, another
path rejected, no normalisation) because every other call site stands in for that predicate
with a double. Threads: #1932 (comment)
and #2434 (comment).
Second review round on the shipped head: a rejected buffer restore or save now holds the artifact sweep as
firmly as a refused one does, so the placeholder and the created directories stay owned and on disk and the
directory snapshot goes back to the session - which is what tells the caller-side guard to keep the session
for a retry instead of resetting it away. Only a confirmed buffer outcome lets the sweep run, so a tab-close
failure on an already-restored buffer still removes the artifacts. The contract test that pinned the old
behaviour (sweep anyway when applyEdit rejects) is replaced by tests for both rejected awaits, and the
owned-placeholder release test now asserts the released state through the public ownsPlaceholder getter.
Threads: #1932 (comment) and
#1932 (comment).
Third review round, filed against the previous head on the sibling pull (#2434 discussion_r4546267986, Minor): a sweep that stops partway - the placeholder
unlink is its first step - left the created directories on disk but untracked, because this unit clears the field up front and only ever handed the snapshot
back on the unconfirmed-buffer path. The retained session then forgot them and a retry could only ever remove the file. The catch now hands the snapshot back
on that path too; the helpers tolerate whatever a partial sweep already removed, so the next discard finishes the sweep in reverse order. Where only a
directory removal failed, ownership is already released and the caller-side reset clears the field again, so that path is unchanged. A regression makes the
unlink fail with EPERM once, keeps the session, and asserts that the retry removes both directories in reverse order and releases the placeholder.1 parent 53cb5b5 commit e80d192
2 files changed
Lines changed: 1867 additions & 59 deletions
0 commit comments