Skip to content

fix(studio-server): history copies a changing file without stalling its sweep - #5627

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/history-copy-stops-at-real-end
Oct 15, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
fix/history-copy-stops-at-real-end

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

History copies every changed project file into its blob store with fs.copyFile. When the copy is a byte copy (no clone) and another writer shortens the file while it is being copied, fs.copyFile never returns: libuv's copy loop asks for the remaining bytes past the new end of the file, gets 0 back, and loops forever. The sweep waits on that copy, and every history operation queues behind the sweep, so undo, redo and the next sweep hang with it. The stuck copy also holds a threadpool thread for good.

A byte copy is what Linux ext4 does (CI runners included), and what macOS does on any volume that is not APFS. On xfs, btrfs and APFS the copy is a clone, which is one atomic snapshot, so the bug never shows there.

What changes

  • blobStore.put copies a project file through cloneOrStream: it first tries a forced clone (COPYFILE_FICLONE_FORCE), which is still one snapshot. Where the file system cannot clone, it streams the file instead, and a stream stops at the file's real end. A file shortened mid-copy is stored as whatever was read, and the next sweep sees the file's new stat and stores its real contents.
  • The stream copy keeps the source's mode, as copyFile did, so Undo still restores an executable file with its exec bit.
  • A copy that fails (a missing, unreadable or changing source) removes its temp file, so no incoming-* file is left in the store.
  • cloneOrCopy is unchanged: it copies blobs out of the store, and those never change.

What I measured

  • Plain Node, no repo code (node 22.22.1, libuv 1.51.0 on Linux; node 22.23.3, libuv 1.52.1 on macOS): a 64 MB file shortened while fs.copyFile copies it is still copying after 5 s on ext4 (8/8 runs, with and without COPYFILE_FICLONE), on tmpfs (12/12) and on a non-clone copy on macOS (2/2). On xfs the copy clones and returns every time (14/14).
  • The new test on a Linux ext4 temp dir: fails on main 3/3 with "the sweep never returned"; passes 3/3 at this head. On xfs it passes on main and at this head (the clone path).
  • The whole src/history suite at this head: 178 passed, 2 skipped, on ext4 and on xfs. The three copy tests pass 3 runs in a row on ext4. tsc --noEmit, oxlint, oxfmt --check (0.41.0) and the comment ratchet are clean.

The test

"a file shortened while the history copies it in" writes a 64 MB file, starts a flush, and waits on the real condition (the store's temp copy passing 1 MB, or the sweep finishing first on a disk that clones). Only then does it shorten the file. It fails within 10 s instead of hanging the run.

Fails without the fix

  • fails without the fix: with main's blobStore.ts, the new test fails on ext4 3/3 with "the sweep never returned".
  • fails without the fix: without the temp cleanup, "leaves no partial copy in the store" fails (an incoming-* file is left).
  • fails without the fix: streaming without the source's mode, "keeps the file's mode" fails on ext4 (644, not 755).

What I did NOT exercise

  • Windows. libuv's Windows copy does not offer a forced clone, so Windows now always takes the stream; I did not run this there.
  • Upstream libuv: the copy loop itself is not fixed here.

@terencechotai2-commits terencechotai2-commits left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict at 9705d48: needs changes. The fix is right, but the PR breaks CI. The blobStore change fixes the hang, and I reproduced both the hang and the fix. The PR also carries two lines of formatter churn in projectHistory.test.ts that fail format:check. That failure takes down Preflight, and with it the two required Windows checks. Revert those two lines and I'd approve.

Blockers

  1. Unrelated reformat of projectHistory.test.ts turns required CI red.
    • Lines 845 and 1389 change Date.now() < until; ); to Date.now() < until;);. The repo's oxfmt 0.41.0 wants the original form back. Locally, npx oxfmt --check packages/studio-server/src/history/projectHistory.test.ts fails, and running oxfmt rewrites exactly those two lines.
    • In CI, Format and Preflight (lint + format) fail with "Format issues found in … projectHistory.test.ts". Preflight gates the Windows lanes, so they are skipped (LANE_RESULT: skipped). The required checks Tests on windows-latest and Render on windows-latest then fail after 4 s.
    • This also matters on its own: Windows is the platform where snapshot always takes the stream path, because libuv has no forced clone there. So the Windows lanes are the only CI coverage of that path, and right now they never ran.
    • Fix: revert the two lines, or run bun run format.

Lows

  1. A failed put now leaves an incoming-* temp file in blobs/.

    • Cause: createWriteStream(to) opens (creates) the destination before the read stream's open error reaches pipeline. put never removes temp when snapshot throws.
    • The common trigger is a project file deleted between the sweep's listing and its copy. That race is routine: storeIfPresent treats ENOENT as "removed, the next sweep records it".
    • I probed openBlobStore directly on ext4 and on xfs:
      • put(missing) rejects ENOENT and leaves a 0-byte incoming-<uuid>.
      • The same happens for EISDIR and for an unreadable (EACCES) source.
      • A read error mid-stream would leave a partial file.
    • At the base, copyFile opens the source first, so the ENOENT case left nothing (checked: dst exists false). This is new on every file system, including the clone ones, because the FICLONE_FORCE attempt fails ENOENT and then falls through to the stream.
    • These files are never read as blobs: openBlobStore gets ENOTDIR from readdir on them and skips them. But nothing counts or prunes them, so they pile up for the life of the history. Suggested fix:
          try {
            await snapshot(absPath, temp);
          } catch (error) {
            await rm(temp, { force: true });
            throw error;
          }
    
  2. The Comments check is red (not required). The comment ratchet reports that blobStore.ts comment share rose from 6.5% to 7.7% because of 2 added lines (the snapshot doc comment). Trimming it to one line, or folding it into the name, clears the check.

  3. Note: on the stream path, blobs now get mode 0644 instead of the source file's mode. copyFile copied the mode; createWriteStream uses 0666 masked by umask. Probe: a 0755 source gave a 755 copy with copyFile and 644 with the stream. writeTo copies the blob's mode onto the restored file, so on non-clone disks Undo now restores an executable file without its exec bit. Mode was already lossy, because blobs are deduped by content. Project files here are html/css/media, so I'm only noting it.

Checked and fine

  • Root cause. On ext4 (and tmpfs), libuv's byte-copy loop in fs.copyFile never returns once the source is shortened under it. The sweep awaits that copy, and every queued history operation waits behind the sweep. Plain COPYFILE_FICLONE doesn't help, because it silently falls back to the same byte copy. FICLONE_FORCE either clones atomically or throws, and the stream path stops at the real EOF.
  • Every path settles. pipeline() rejects and destroys both streams on any read error, open error or write error, so there is no 'finish'-style wait left. I walked these cases:
    • Shortened: the read returns 0, EOF, and the stream resolves.
    • Grown: the stream reads to the new end and resolves. The probe stored 72 MB from a 64 MB file grown mid-copy, and the hash matched.
    • Deleted or renamed: the open fd keeps reading the old inode.
    • Source EACCES/EISDIR, destination EACCES: these reject with the right code, so storeIfPresent's ENOENT handling still works.
    • fd count: 23 before and 23 after all the error cases, so no leaked descriptors.
    • Unchanged by this PR: a source that never stops growing would keep the stream reading, and concurrent puts of the same content each use their own temp and rename onto the same hash path.
  • Data integrity.
    • The hash is still taken from the copy. A torn copy (a writer mid-rewrite) is stored under its own correct hash, never under the source's.
    • The sweep records the listing stat taken before the copy, or "" when that stat is within RACY_MS. A file changed during the copy therefore shows a new stat on the next sweep and is stored again.
    • On a non-clone disk, the timeline can briefly hold an intermediate version. The old byte copy had the same behaviour for in-place writes that didn't shorten the file.
    • The temp file sits in blobs/, on the same file system as blobs/xx/<hash>, so the final rename is atomic. Partial temps are never renamed into place, because rename runs only after snapshot resolves.
    • cloneOrCopy and writeTo are unchanged and only read immutable blobs, so Undo writes whole blobs through temp+rename as before.
  • Clone fallback. A failed FICLONE_FORCE leaves no destination behind (libuv unlinks it), and createWriteStream truncates the file anyway.
  • Speed. Copying 1 GB on this ext4 box took 3.1 s with the stream and 9.3 s with copyFile, so the stream path is no slower.
  • The test.
    • It reproduces the hang. With the base blobStore.ts on an ext4 TMPDIR it failed 2/2 with "the sweep never returned", and it passes at the head.
    • On xfs (/tmp) it passes on the base too, because that disk takes the clone path, as the PR body says. Ubuntu runners use ext4, so CI exercises it.
    • Mutation: swapping COPYFILE_FICLONE_FORCE for COPYFILE_FICLONE in snapshot makes it fail with "the sweep never returned". Always taking the stream path passes, which is expected because that is also a valid fix.

Checks run

  • vitest run src/history/projectHistory.test.ts -t shortened:
    • at the head on ext4: pass;
    • with the base blobStore.ts on ext4: fail 2/2 ("the sweep never returned");
    • with the base on xfs: pass;
    • with the FICLONE (non-force) mutation on ext4: fail;
    • with the always-stream mutation: pass.
  • Full src/history Vitest suite at the head: 176 passed, 2 skipped, on both ext4 and xfs.
  • tsc --noEmit in packages/studio-server: clean. oxlint on both changed files: clean. oxfmt --check: fails on projectHistory.test.ts (Blocker 1).
  • A scratch probe of openBlobStore().put covering a missing, directory and unreadable source, a grown source, a read-only store dir, and an fd count. It was deleted afterwards.
  • A plain-Node probe of base copyFile ENOENT leftovers, file mode, and 1 GB copy timing.
  • gh pr checks --required: 8 pass. Tests on windows-latest and Render on windows-latest fail because their lanes were skipped after Preflight failed on format. Not required: Format, Preflight, Comments and preview-regression are red, and the Studio edit-accuracy shards were still pending.

— tai2

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 164 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • textedit-none-px-r0-nested-z100: tracking 0, pressJump -, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0, pressJump -, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0, pressJump -, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Thanks, tai2. All four are fixed at the new head.

Blocker (format). The two until; ); lines are back as they were; oxfmt --check (0.41.0) is clean on both files.

Low 1 (partial temp). put owns the temp, so it removes it when the copy throws. The stream path also stats the source first, so a missing source throws ENOENT before any temp exists. New test, "leaves no partial copy in the store": a missing source (ENOENT) and an unreadable one (EACCES) both leave blobs/ empty. It fails without the cleanup. It is skipped on Windows and as root, where mode 000 does not block a read.

Low 2 (comments). The doc comment is gone, and the name says it: cloneOrStream. The ratchet is at or under the base.

Low 3 (mode). Fixed rather than noted: the stream opens its copy with the source's mode, as copyFile did. New test, "keeps the file's mode": a 0755 file is stored as 0755. On ext4 it fails without the fix (644).

On a Linux machine: the three copy tests pass 3 runs in a row on ext4, the src/history suite is 178 passed / 2 skipped on ext4 and xfs, and tsc and oxlint are clean.

@terencechotai2-commits terencechotai2-commits left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving f611743c. My blocker and the lows from 9705d489 are fixed. The delta is two commits on top of 9705d489 (60132f66, f611743c), touching only blobStore.ts and projectHistory.test.ts.

  • Blocker (format churn): fixed. Lines 845 and 1389 are back to until; );, and oxfmt --check is clean on both files. Preflight no longer stops the Windows lanes, so CI now covers the stream path there.
  • Partial copy left on failure: fixed. A failed cloneOrStream now removes incoming-* before rethrowing. The new test "leaves no partial copy in the store" covers both a missing source (ENOENT) and an unreadable one (EACCES). Dropping the .catch(rm) fails it.
  • Mode on the stream path: fixed. The stream copy now creates the temp file with the source's mode. The new test "keeps the file's mode" checks 0o755. Note: on this box /tmp is xfs, so the copy always clones and keeps the mode by itself. Dropping { mode } passed locally for that reason. The ext4 Linux CI runner takes the stream path, so the test means something there.
  • Comments check: the explanatory doc comment on the copy helper was dropped and the helper renamed cloneOrStream. I'd have kept one line on why copyFile alone can hang, but that's not blocking.

Checks run

  • vitest run src/history at f611743c: 178 passed, 2 skipped.
  • oxfmt --check on both changed files: clean.
  • Mutants: dropping the cleanup .catch was caught. Dropping { mode } was not caught here, because xfs clones.
  • gh pr checks --required at this head: green before posting, including both Windows checks.

— tai2

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 916ec37 Oct 15, 2026
104 of 105 checks passed
@miguel-heygen
miguel-heygen deleted the fix/history-copy-stops-at-real-end branch October 15, 2026 03:30
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.

2 participants