[jmandel]

Every package the crawler stores, from any feed, can briefly be served as a missing or partial tarball. This PR makes sure an advertised id#version only ever has a complete file behind it.

The problem

In packages/package-crawler.js the crawler:

1. commits the package to the database, after which the registry advertises it and redirects downloads (bucketPath) to <id>-<version>.tgz in the mirror, which nginx serves at /web/ and the CDN fetches on a miss;
2. then writes the mirror file with fs.writeFileSync under its final name, which truncates and rewrites in place.

That gives two windows:

• Every new package, any source: between the commit and the end of the write, the package is listed but its file is missing (a 404, which the CDN caches briefly) or partly written. A partial file can look like a complete response and be cached by the CDN for up to a day. The window is short, but it opens on every publication, which is exactly when tools fetch the new version.
• A direct copy replacing a Simplifier copy: a file that has been served for a while is truncated and rewritten in place, so a download in progress, or one arriving during the rewrite, gets a short or mixed file.

The change

The new tarball is written beside its target as <file>.tmp-<pid>, which never ends in .tgz and so is never served. It is then renamed onto the real name, and the step order depends on the case:

• New package: rename into place, then commit. The file is complete before anything can ask for it.
• Replacement: commit, then rename. The old file stays fully served until the replacement has committed, and then the name switches to the new file in one step.
• If the commit fails: the served file is untouched, the temp file is removed, and a new package's file is removed too, since it was never advertised.

On Linux, rename within one directory is atomic, and a reader that already has the old file open keeps reading it to the end. So readers see either the old file or the complete new one, never a mixture.

Tests

New cases in tests/packages/package-crawler-versions.test.js:

• New package: the mirror holds the complete new file at the moment of commit. This one fails on current main.
• Replacement: at commit time the mirror still holds the old file, and afterwards it holds the new one.
• Failed commit: a replacement leaves the served file alone, and a new package leaves no file.
• Earlier test, kept: a reader that had the old file open still reads all of it.

All 176 tests in tests/packages pass.

🤖 Generated with Claude Code

[grahamegrieve]

This one makes my skin crawl, honestly. For the following reasons:

• there's no delay. all my experience with write/rename is that file locking issues are going to interfere with the rename
• if the file already exists, what happens to the rename? It doesn't fail if there's a read lock on the file?
• the test case tests behaviour on the development system, not the production system. So what does it prove?
• this is a super corner case anyway - simplifier is read last, so how does simplifier get the feed item before the crawler?

the last cuts both ways, I guess. But this change just feels super-under done to me

[jmandel]

Thanks — fair questions. Short answers, since production here is Linux:

• Locks and the delay. On Linux, rename(2) within one filesystem replaces the directory entry atomically, and file locks are advisory: nothing blocks the rename. A reader that already has the file open (nginx mid-download) keeps reading the old inode to the end; the old data is freed when it closes. New opens get the new file. So there's no window to wait out, and no race for a delay to fix.
• Rename over an existing file. It succeeds and replaces the target, even while someone is reading it. That's the standard way to update a served file without readers seeing a partial one. The in-place writeFileSync truncates first, so a reader mid-download gets a short or mixed tarball.
• Windows. This is where your experience is right. There, MoveFileEx (what Node uses) can fail with EPERM/EBUSY if the target is open without delete sharing, and antivirus makes that worse. If the crawler needs to run on Windows, I'm happy to add a short retry, falling back to the old in-place write, on win32 only.
• The test. It checks the property on a POSIX filesystem: an open reader keeps the complete old bytes. It fails on current main and passes with the change. That is the production case (Linux plus nginx), but you're right that it says nothing about Windows.
• How often. Within one crawl Simplifier is read last, so this happens across crawls: a package first picked up via Simplifier (because its direct feed was late or down that run) is later replaced by the direct copy ("Replaced Simplifier copy" in the crawl log). It's rare. The reason I bothered is the CDN: a partial tarball fetched on a cache miss would be served to everyone for up to a day.

Happy to add the win32 fallback, or to close this if you'd rather not take it.

[jmandel]

Updated, and you were right that it wasn't robust enough, though not because of the rename itself. The crawler commits a package to the database before writing its mirror file. So for every new package, from any feed and not just the Simplifier case, there was a moment when the registry advertised it but the file was missing or half-written. Write-then-rename alone only turned the partial file into a brief 404.

The new commit fixes the order:

• New package: the file is put in place before the commit.
• Replacement: the new file waits beside the old one, and is renamed over it only after the commit.
• Failed commit: the served file is left alone.

So there's no moment at which an advertised package lacks a complete file. A test for the new-package case fails on current main. The description is rewritten to match.

[grahamegrieve]

ok thanks