fix(install): check cached package.jsons (#14899)

src/install/install.zig

@@ -2341,13 +2341,29 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
                 supported_method;
         }
 
-        pub fn packageMissingFromCache(this: *@This(), manager: *PackageManager, package_id: PackageID) bool {
+        pub fn packageMissingFromCache(this: *@This(), manager: *PackageManager, package_id: PackageID, resolution_tag: Resolution.Tag) bool {
             const state = manager.getPreinstallState(package_id);
             return switch (state) {
                 .done => false,
                 else => brk: {
                     if (this.patch.isNull()) {
-                        const exists = Syscall.directoryExistsAt(this.cache_dir.fd, this.cache_dir_subpath).unwrap() catch false;
+                        const exists = switch (resolution_tag) {
+                            .npm => package_json_exists: {
+                                var buf = &PackageManager.cached_package_folder_name_buf;
+
+                                if (comptime Environment.isDebug) {
+                                    bun.assertWithLocation(bun.isSliceInBuffer(this.cache_dir_subpath, buf), @src());
+                                }
+
+                                const subpath_len = strings.withoutTrailingSlash(this.cache_dir_subpath).len;
+                                buf[subpath_len] = std.fs.path.sep;
+                                defer buf[subpath_len] = 0;
+                                @memcpy(buf[subpath_len + 1 ..][0.."package.json\x00".len], "package.json\x00");
+                                const subpath = buf[0 .. subpath_len + 1 + "package.json".len :0];
+                                break :package_json_exists Syscall.existsAt(bun.toFD(this.cache_dir.fd), subpath);
+                            },
+                            else => Syscall.directoryExistsAt(this.cache_dir.fd, this.cache_dir_subpath).unwrap() catch false,
+                        };
                         if (exists) manager.setPreinstallState(package_id, manager.lockfile, .done);
                         break :brk !exists;
                     }
@@ -2377,13 +2393,8 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
         }
 
         pub fn install(this: *@This(), skip_delete: bool, destination_dir: std.fs.Dir, resolution_tag: Resolution.Tag) Result {
-            // If this fails, we don't care.
-            // we'll catch it the next error
-            if (!skip_delete and !strings.eqlComptime(this.destination_dir_subpath, ".")) this.uninstallBeforeInstall(destination_dir);
-
-            if (comptime kind == .regular) return this.installImpl(skip_delete, destination_dir, this.getInstallMethod(), resolution_tag);
-
             const result = this.installImpl(skip_delete, destination_dir, this.getInstallMethod(), resolution_tag);
+            if (comptime kind == .regular) return result;
             if (result == .fail) return result;
             const fd = bun.toFD(destination_dir.fd);
             const subpath = bun.path.joinZ(&[_][]const u8{ this.destination_dir_subpath, ".bun-patch-tag" });
@@ -12656,7 +12667,6 @@ pub const PackageManager = struct {
             var installer = PackageInstall{
                 .progress = this.progress,
                 .cache_dir = undefined,
-                .cache_dir_subpath = undefined,
                 .destination_dir_subpath = destination_dir_subpath,
                 .destination_dir_subpath_buf = &this.destination_dir_subpath_buf,
                 .allocator = this.lockfile.allocator,
@@ -12803,7 +12813,7 @@ pub const PackageManager = struct {
             this.summary.skipped += @intFromBool(!needs_install);
 
             if (needs_install) {
-                if (!remove_patch and resolution.tag.canEnqueueInstallTask() and installer.packageMissingFromCache(this.manager, package_id)) {
+                if (!remove_patch and resolution.tag.canEnqueueInstallTask() and installer.packageMissingFromCache(this.manager, package_id, resolution.tag)) {
                     if (comptime Environment.allow_assert) {
                         bun.assertWithLocation(resolution.canEnqueueInstallTask(), @src());
                     }

src/install/npm.zig

@@ -35,6 +35,7 @@ const http = bun.http;
 const OOM = bun.OOM;
 const Global = bun.Global;
 const PublishCommand = bun.CLI.PublishCommand;
+const File = bun.sys.File;
 
 const Npm = @This();
 
@@ -1201,28 +1202,28 @@ pub const PackageManifest = struct {
         pub fn loadByFileID(allocator: std.mem.Allocator, scope: *const Registry.Scope, cache_dir: std.fs.Dir, file_id: u64) !?PackageManifest {
             var file_path_buf: [512 + 64]u8 = undefined;
             const file_name = try manifestFileName(&file_path_buf, file_id, scope);
-            var cache_file = cache_dir.openFileZ(
-                file_name,
-                .{ .mode = .read_only },
-            ) catch return null;
+            const cache_file = File.openat(cache_dir, file_name, bun.O.RDONLY, 0).unwrap() catch return null;
             defer cache_file.close();
             return loadByFile(allocator, scope, cache_file);
         }
 
-        pub fn loadByFile(allocator: std.mem.Allocator, scope: *const Registry.Scope, manifest_file: std.fs.File) !?PackageManifest {
-            const bytes = try manifest_file.readToEndAllocOptions(
-                allocator,
-                std.math.maxInt(u32),
-                manifest_file.getEndPos() catch null,
-                @alignOf(u8),
-                null,
-            );
-
+        pub fn loadByFile(allocator: std.mem.Allocator, scope: *const Registry.Scope, manifest_file: File) !?PackageManifest {
+            const bytes = try manifest_file.readToEnd(allocator).unwrap();
             errdefer allocator.free(bytes);
+
             if (bytes.len < header_bytes.len) {
                 return null;
             }
-            return try readAll(bytes, scope);
+
+            const manifest = try readAll(bytes, scope) orelse return null;
+
+            if (manifest.versions.len == 0) {
+                // it's impossible to publish a package with zero versions, bust
+                // invalid entry
+                return null;
+            }
+
+            return manifest;
         }
 
         fn readAll(bytes: []const u8, scope: *const Registry.Scope) !?PackageManifest {
@@ -1311,7 +1312,7 @@ pub const PackageManifest = struct {
                 },
             };
 
-            const maybe_package_manifest = Serializer.loadByFile(bun.default_allocator, &scope, manifest_file) catch |err| {
+            const maybe_package_manifest = Serializer.loadByFile(bun.default_allocator, &scope, File.from(manifest_file)) catch |err| {
                 global.throw("failed to load manifest file: {s}", .{@errorName(err)});
                 return .zero;
             };

src/sys.zig

@@ -3013,14 +3013,14 @@ pub const File = struct {
     // "handle" matches std.fs.File
     handle: bun.FileDescriptor,
 
-    pub fn openat(other: anytype, path: anytype, flags: bun.Mode, mode: bun.Mode) Maybe(File) {
+    pub fn openat(other: anytype, path: [:0]const u8, flags: bun.Mode, mode: bun.Mode) Maybe(File) {
         return switch (This.openat(bun.toFD(other), path, flags, mode)) {
             .result => |fd| .{ .result = .{ .handle = fd } },
             .err => |err| .{ .err = err },
         };
     }
 
-    pub fn open(path: anytype, flags: bun.Mode, mode: bun.Mode) Maybe(File) {
+    pub fn open(path: [:0]const u8, flags: bun.Mode, mode: bun.Mode) Maybe(File) {
         return File.openat(bun.FD.cwd(), path, flags, mode);
     }
 

test/cli/install/registry/bun-install-registry.test.ts

@@ -3056,6 +3056,69 @@ describe("binaries", () => {
   });
 });
 
+test("it should invalid cached package if package.json is missing", async () => {
+  await Promise.all([
+    write(
+      packageJson,
+      JSON.stringify({
+        name: "foo",
+        dependencies: {
+          "no-deps": "2.0.0",
+        },
+      }),
+    ),
+  ]);
+
+  let { out } = await runBunInstall(env, packageDir);
+  expect(out).toContain("+ [email protected]");
+
+  // node_modules and cache should be populated
+  expect(
+    await Promise.all([
+      readdirSorted(join(packageDir, "node_modules", "no-deps")),
+      readdirSorted(join(packageDir, ".bun-cache", "[email protected]@@localhost@@@1")),
+    ]),
+  ).toEqual([
+    ["index.js", "package.json"],
+    ["index.js", "package.json"],
+  ]);
+
+  // with node_modules package.json deleted, the package should be reinstalled
+  await rm(join(packageDir, "node_modules", "no-deps", "package.json"));
+  ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false }));
+  expect(out).toContain("+ [email protected]");
+
+  // another install is a no-op
+  ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false }));
+  expect(out).not.toContain("+ [email protected]");
+
+  // with cache package.json deleted, install is a no-op and cache is untouched
+  await rm(join(packageDir, ".bun-cache", "[email protected]@@localhost@@@1", "package.json"));
+  ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false }));
+  expect(out).not.toContain("+ [email protected]");
+  expect(
+    await Promise.all([
+      readdirSorted(join(packageDir, "node_modules", "no-deps")),
+      readdirSorted(join(packageDir, ".bun-cache", "[email protected]@@localhost@@@1")),
+    ]),
+  ).toEqual([["index.js", "package.json"], ["index.js"]]);
+
+  // now with node_modules package.json deleted, the package AND the cache should
+  // be repopulated
+  await rm(join(packageDir, "node_modules", "no-deps", "package.json"));
+  ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false }));
+  expect(out).toContain("+ [email protected]");
+  expect(
+    await Promise.all([
+      readdirSorted(join(packageDir, "node_modules", "no-deps")),
+      readdirSorted(join(packageDir, ".bun-cache", "[email protected]@@localhost@@@1")),
+    ]),
+  ).toEqual([
+    ["index.js", "package.json"],
+    ["index.js", "package.json"],
+  ]);
+});
+
 test("it should install with missing bun.lockb, node_modules, and/or cache", async () => {
   // first clean install
   await writeFile(