Skip to content

fix(core): write back no file moved or deleted during a folder pass - #2322

Merged
aicayzer merged 5 commits into
mainfrom
fix/core-folder-pass-writeback
Oct 23, 2026
Merged

aicayzer merged 5 commits into
mainfrom
fix/core-folder-pass-writeback

Conversation

@aicayzer

@aicayzer aicayzer commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A folder pass scans, sends, then pulls. When a person moved, renamed or deleted a bound file between the scan and the pull, the pull wrote the file back at its old path whenever the item's render differed from the bytes the scan read, which is always true for a new note's first marfa_id write-back and for any remote change. The next scan then saw the moved file as a copy and created a second item, or brought a deleted file back. The pull now writes back no bound file that is missing and not journaled; the next scan follows the move or journals the delete, and the existing grace rules decide from there.

Closes #2321

Changes

  • Root cause. The scan journals only what it sees missing, so a file that vanished after the scan had no journal row. In write_placed, the still-check treated a missing file as unchanged, in_place came out false, and land wrote the render at the old path.
  • Fix (core/marfa-core/src/folder/pull.rs). The still-check now separates three cases: changed since the scan (unchanged behavior), unreadable (now reported with an honest reason instead of the "changed" one), and missing. A missing file that is neither journaled nor marked put back by a restore is counted unwritten with the reason "the file is gone since the scan read it, and the next scan tells whether it moved". A file whose render already matches what the scan read is taken as in place only while it is present (a stat, no read), so a file the pull placed or revived itself and the person then moved or deleted is counted unwritten, not unchanged.
  • Paths no scan reaches. A bound file gone from a path the lists no longer take, or inside a nested folder, is written where its item wants it, as before this change: no scan reaches that path, so none could tell a move from a delete. The scan and the pull share one check for this (Folder::reaches).
  • Restore keeps working. removal.rs exposes its PUT_BACK sentinel to the pull, so a restore of a paused removal still writes its missing files back.
  • Delete before the first write (deleted_as_agreed). A new note deleted before the folder first wrote marfa_id, type and tier into it was revived once journaled, because the render's added lines read as a change made elsewhere. Lines the agreed presentation never had are now left out of the comparison, so the journaled delete stands.
  • Test hook (watch.rs). A debug-only folder-pass-pull hold between a pass's drain and its pull, documented in conformance/README.md.
  • Contract (conformance/spec/folders.md). New folders/pull-missing-file-not-written (limited to paths the lists take outside a nested folder), folders/pull-missing-file-unwritten-count, folders/pull-missing-file-moved-keeps-item and folders/delete-before-first-write; a new test cited under folders/delete-remote-revives.
  • Changed existing test. folders.test.ts › holds the files of a directory it cannot read, and goes on with the rest pinned the old behavior: after an incomplete scan, the pull wrote a deleted file back. It now asserts the file stays gone, is counted unwritten, is journaled by the next complete scan and is deleted after the grace. No contract rule required the write-back.

Testing

Red first: the new tests were run on main (889da4d) with only the tests and the hold point added.

  • Core: cargo test -p marfa-core --lib folder:: gave 3 failed, 2 passed. The moved, deleted and revived cases failed with PullReport { rewritten: 1, .. }; the two witnesses (a present file changed elsewhere is written in place, and an unreadable file is not written over) passed, as intended.
  • Device: all 9 new device tests failed, for example "the pull wrote a moved file back at its old path, so the note is in the folder twice: expected true to be false", and the burst case "expected +0 to be 4".

Red first for the review's finding, on this branch at 51dc50c with only the new tests added:

  • Core: a_file_the_pull_wrote_and_then_gone_since_the_scan_is_unwritten_not_unchanged failed with Some(None): PullReport { unchanged: 1, unwritten: 0, flagged: [], .. }, left: (1, 0), right: (0, 1).
  • Device: counts a file the pull wrote itself and deleted after the scan as unwritten, not unchanged failed with expected [ +0, 2 ] to deeply equal [ 1, 1 ].
  • Core (folder/pull.rs): a_file_gone_from_a_path_no_scan_reaches_is_written_where_its_item_wants_it failed with the reach check removed (unwritten: 1, nothing written at a.md); its witness, the same path still taken by the lists, is held back.

Green on this branch:

  • cargo fmt --all, cargo clippy -p marfa-core --all-targets -- -D warnings: clean.
  • cargo test --workspace in core/: 1400 passed, 0 failed. One earlier run had replace_tests::a_store_another_process_writes_is_refused_as_held_and_never_replaced fail once at its witness (replace_refused after the lock is dropped); it passed in 3 runs alone and 2 more full runs. That test is untouched by this change.
  • Device suites, vitest run --project device src/suites/device/folders src/suites/device/binding-folders: 21 files, 793 tests passed. This run also caught the restore regression in a first version of the fix (7 restore and unreadable tests), fixed by the PUT_BACK exemption above.
  • src/suites/device/folder-*: 48 passed; folder-settings-live.test.ts needs a live server (MARFA_API_URL) and was not run.
  • spec-form.test.ts and spec-citations.test.ts: 34 passed.
  • After pnpm build: pnpm format:check and pnpm lint clean; pnpm test:changed passed (88 files, 1144 tests, then 24 files, 238 tests).

New tests:

  • Core (folder/mod.rs): a plain move, a move into a subdirectory and a move with an unsent edit; a delete that stays deleted and reaches the bin after the grace (written and never-written notes); a delete revived by a change it shows once journaled; an unreadable file; a present file changed elsewhere (witness).
  • Core (folder/mod.rs): a file the pull revived, then deleted or moved after the scan, is unwritten; kept in place, it is unchanged (witness).
  • Core (folder/pull.rs): a file gone from a path the lists no longer take is written where its item wants it; the witness with the path still taken holds it back.
  • Device (folders-pass-writeback.test.ts): a plain move, a rename with a title-line edit, a delete, a move with an unsent edit, a burst of two moves and two deletes, a witness that writes a remote change in place and revives a deleted file once journaled, and a revived file deleted after the scan counted unwritten beside a file in place counted unchanged.
  • Device (folders-one-mac.test.ts): a move to another folder on the same Mac.
  • Device (binding-folders-clock.test.ts): a move and a delete in a real watch pass, held at folder-pass-pull, with the manual clock driving the grace.

Not covered:

  • A file moved to another folder while a remote change to its item arrives inside the grace. The old folder revives it after journaling, so the note can appear in both folders. This was there before this change; the pull does not consult other folders before the grace ends.
  • A file without marfa_id moved to another folder before its first write-back becomes a new item there, as before.
  • In the offline core test, the rename scan sends the whole file body over an unpulled remote change; the server's merge of that was not exercised here.

Notes for reviewers

  • Deviation from the brief. The pull never writes back a missing file that is not journaled, even when its item changed elsewhere. The revive comes one pass later, after the scan journals the file, still inside the grace. Writing it back at once would duplicate a file that was moved while a remote change arrived.
  • Other paths checked: incomplete scans (no journal, so held back), unreached paths (written where the item wants, as above), a gone root, a missing parent directory, settle_landings, the waiting second round, move_bound, restore and rebinding (no binding), blob placements, departing, let_go, take_away, retitle_file and behind_by_its_line_alone.
  • Docs: docs(guides): say a running sync never writes back a moved or deleted file docs#592 updates Folder safeguards.
  • Review depth: full. This changes when a folder writes a person's files and when it sends deletes.

Checklist

  • The title is a scoped Conventional Commit.
  • New behavior has a test that was seen failing first, or no test was needed and the reason is above.
  • Docs, comments and the contract still say what is true after this change.

The in-place check took a bound file whose render matched the scanned bytes as unchanged without looking at the disk, so a file the pull had placed or revived and the person then moved or deleted was reported unchanged. The check now requires the file to be present.

A file gone from a path no scan reaches (the lists stopped taking it, or a nested folder) is written where its item wants it again, since no scan can tell its move from a delete; the scan and the pull share one reach check.

@aicayzer aicayzer left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ready to merge with open items

Summary. A bound file that is missing when a pull reaches its item is no longer written back, so a move or delete during a pass is not undone; the one blocking finding is fixed and re-tested, and the deviation from the first acceptance criterion is stated below.

First draft: 3/5. 1 blocking issue found (a file the pull wrote itself and that was then deleted or moved was counted unchanged, contradicting folders/pull-missing-file-unwritten-count), fixed in d62e7a1 with a core test and a device test, each seen failing first.

Checked.

  • Done: the acceptance criteria against the tests; the red run reproduced with main's pull.rs under the branch's tests; witnesses in every absence test.
  • True: the new contract statements against the chapter's form and their cited tests; the Folder safeguards, Sync a folder and Keep folders running pages.
  • Every path: the still-check, restore, journaled revive, incomplete scan, gone root, settle_landings, move_bound with a missing source, departing, take_away, blob reads and rebinding. No other pull path reads an error or absence as unchanged.
  • Keep what people made: a file moved with an unsent edit keeps and sends the edit.
  • Real conditions: the folder-pass-pull hold is debug-only like the other holds; the new device tests drive scan, drain and pull as explicit steps.
  • Changed existing test: folders.test.ts "holds the files of a directory it cannot read" pinned the write-back of a file deleted during an incomplete scan; no statement requires that.
  • The fixes for the findings (d62e7a1, f16190f) were checked by the session by reading the diff and by the builder's runs; the reviewer did not re-review them.

Found.

  • A missing file the pull wrote itself was counted unchanged. Fixed in d62e7a1: the in-place shortcut now requires the file on disk. True, Report only what is known.
  • A path no scan reaches got a reason that was not true there. Fixed in d62e7a1: the hold applies only to paths the scan reaches (lists take them, outside any nested folder); elsewhere the item is written where it wants to be, as before. The reason text and folders/pull-missing-file-not-written are narrowed to match. Report only what is known.
  • The first acceptance criterion is met one pass later. A missing, unjournaled file is never written in the same pass, even when its item changed in content the file shows; it comes back on the next pass, once the scan journals it and still inside the grace. Writing it at once would duplicate a file moved while a remote change arrived. Accepted. Done.

For an issue.

  • A file moved to another folder while a remote change to its item arrives inside the grace can still be revived by the old folder, so the note can appear in both. Existing behavior, rare.
  • An unexplained failure of replace_tests::a_store_another_process_writes_is_refused_as_held_and_never_replaced (witness line) was seen once in a full run and not again in 5 runs; cause not found.

Quick fixes. 1 applied by the reviewer, in withmarfa/docs#592.


Reviewed by withmarfa-reviewer on Claude Fable (first round), fixes checked by the session. This review may contain mistakes.

@aicayzer
aicayzer marked this pull request as ready for review October 23, 2026 01:56
@aicayzer
aicayzer enabled auto-merge (squash) October 23, 2026 01:56
@aicayzer
aicayzer merged commit 41fc3dd into main Oct 23, 2026
53 checks passed
@aicayzer
aicayzer deleted the fix/core-folder-pass-writeback branch October 23, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A folder pass writes back a file that was moved or deleted during the pass

1 participant