Merged
Conversation
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.
Closes #1650
pasteover an existing file now copies the outgoing bytes to~/.cache/supertool/paste-backup/before the write, and names the copy in the receipt.Not the guard the issue proposed, and the argument is the point
I filed #1650 asking for a refusal above some shrink ratio or byte-loss floor, with a
forcetoken. The agent refused all three and it is right:force, andpasteover an existing file is documented and ordinary. Soforcegets typed by reflex, and a guard everyone routes around has deleted itself. This repo already has the write-up on that failure mode under themisdirectsclass.pastereplaces the whole file. The one number in the diff — an 8 MB copy bound — is a resource limit, not a trigger, and crossing it is disclosed rather than silent.A copy has no case it was not written for: the write happens either way, and a false positive costs one cache file.
Scope is
pastealone, and that is complete for this defectProbed rather than assumed. Every other mutating op fails on a path that is not there —
edit/replacematch a string first,vimandreplace_linesreturnfile not found. Onlypastesucceeds either way. Plenty else destroys bytes (vim ggdGempties a file;replace_linesclampsend == total+1rather than refusing it) — but nothing else can destroy a file the caller believes is not there, which is the #1642 mechanism. A test pinsvimrefusing a missing path, so a future create-if-missing arm cannot open that hole silently.Reviewer: 5 accepted, 4 refused
The one that mattered:
test_the_store_is_reapedwould have passed against a guard that did nothing. Rewritten end to end — real snapshot,utime8 days stale,gc:run:paste-backup, assert gone and1 removed.Three reviewer claims were checked and found false before being refused (
_supertool.pyis not the largest file in the tree —CHANGELOG.mdis 1.8 MB;vim's missing-path and diff-echo cases are unrelated;replace_linesclamps rather than refuses).Known, documented, not fixed here
A
pasterolled back by a validator still leaves a copy, and the receipt printsprevious contents kept at PATHabovethe file was NOT edited. Both sentences are true. Fixing it means reaching into the shared rollback loop, so it is documented inedits.mdand the fragment instead.Windows
The core adds no subprocess and no separator logic. The tests had the platform defect and it was caught pre-commit: every
write_text/read_textbecamewrite_bytes/read_bytes, because CRLF translation would have made an 8922-byte note 8923 on Windows and turned the identical-rewrite case into a differing one — the #1004 class exactly. Core behaviour on Windows is reasoned, not observed; CI is the authority.