From 5078b8a87475a36d61af29564feb303a5f36b39d Mon Sep 17 00:00:00 2001
From: Sabyasachi <saby@stackbox.xyz>
Date: Fri, 14 Aug 2026 12:09:49 +0000
Subject: [PATCH 1/3] fix(file-store): make read failures reject instead of
 surfacing late
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

GCS getAsStream returned createReadStream() without any I/O, so a missing object
resolved successfully and failed later as a stream 'error'. A caller doing
reply.send(await getAsStream(k)) sent a 200 with a broken body, or crashed the
process on an unhandled error. Every other provider rejects. GCS is the production
backend. It now checks existence first.

Plugin dispatch read Configure[opts.type] straight off an object literal, so
type:"toString" resolved Object.prototype.toString and registered successfully with
no FileStore decorated — a typo became a silent no-op boot rather than a failure.

streamToBuffer only handled data/error/end. A stream destroyed without an error
emits close alone, leaving the promise pending forever and hanging the request that
awaited getAsBuffer. It now rejects, and a settled guard keeps late events from
settling twice.

Local getAsBuffer/getAsStream wrapped reads in if (await stat(p)), which can never be
falsy since stat throws — so the File not found branch was unreachable and callers
got raw ENOENT. Two specs reached it only by mocking stat to resolve undefined, a
state the real filesystem never produces, and asserted an error the code cannot throw.
getAsBuffer now reads directly; getAsStream keeps the stat so a missing file rejects
rather than deferring to a stream error, matching the GCS fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/file-store.gcs.spec.ts   | 12 ++++++++
 src/file-store.local.spec.ts | 32 +++++++++++++-------
 src/file-store.ts            | 31 ++++++++++++-------
 src/utils.spec.ts            | 58 +++++++++++++++++++++++++++++++-----
 src/utils.ts                 | 24 ++++++++++++---
 5 files changed, 125 insertions(+), 32 deletions(-)

diff --git a/src/file-store.gcs.spec.ts b/src/file-store.gcs.spec.ts
index e423b18..7a48193 100644
--- a/src/file-store.gcs.spec.ts
+++ b/src/file-store.gcs.spec.ts
@@ -118,10 +118,22 @@ describe("GCPFileStore", () => {
   describe("getAsStream", () => {
     it("should return the read stream", async () => {
       const rs = Readable.from(["data"]);
+      mockFile.exists.mockResolvedValueOnce([true]);
       mockFile.createReadStream.mockReturnValueOnce(rs);
 
       expect(await store.getAsStream("test.txt")).toBe(rs);
     });
+
+    // createReadStream does no I/O, so without an existence check a missing object
+    // resolves and fails later as a stream 'error'. Every other provider rejects.
+    it("rejects for a missing object instead of returning a doomed stream", async () => {
+      mockFile.exists.mockResolvedValueOnce([false]);
+
+      await expect(store.getAsStream("missing.txt")).rejects.toThrow(
+        "File not found: missing.txt",
+      );
+      expect(mockFile.createReadStream).not.toHaveBeenCalled();
+    });
   });
 
   describe("copyFromStream", () => {
diff --git a/src/file-store.local.spec.ts b/src/file-store.local.spec.ts
index cf03d90..afec7ac 100644
--- a/src/file-store.local.spec.ts
+++ b/src/file-store.local.spec.ts
@@ -75,6 +75,19 @@ describe("LocalFileStore", () => {
         fastify.register(FileStorePlugin, { type: "nope" as any }),
       ).rejects.toThrow("Unknown storage type: nope");
     });
+
+    // A plain lookup finds Object.prototype.toString and registers with no FileStore
+    // decorated — a typo becoming a silent no-op boot instead of a failure.
+    it.each(["toString", "constructor", "valueOf", "hasOwnProperty"])(
+      "rejects the inherited Object.prototype name %s",
+      async (type) => {
+        const f = Fastify({ logger: false });
+        await expect(
+          f.register(FileStorePlugin, { type: type as never }),
+        ).rejects.toThrow(`Unknown storage type: ${type}`);
+        await f.close();
+      },
+    );
   });
 
   describe("exists", () => {
@@ -165,12 +178,11 @@ describe("LocalFileStore", () => {
       expect((await store.getAsBuffer("read.txt")).toString()).toBe("buffered");
     });
 
-    it("throws File not found when stat yields nothing", async () => {
+    it("rejects with ENOENT for a missing file", async () => {
       const store = await register();
-      jest.spyOn(fs.promises, "stat").mockResolvedValue(undefined);
-      await expect(store.getAsBuffer("ghost.txt")).rejects.toThrow(
-        `File not found: ${path.join(tempDir, "ghost.txt")}`,
-      );
+      await expect(store.getAsBuffer("ghost.txt")).rejects.toMatchObject({
+        code: "ENOENT",
+      });
     });
   });
 
@@ -182,12 +194,12 @@ describe("LocalFileStore", () => {
       expect((await streamToBuffer(rs)).toString()).toBe("streamed");
     });
 
-    it("throws File not found when stat yields nothing", async () => {
+    // Rejects rather than deferring to a stream 'error': createReadStream is lazy.
+    it("rejects with ENOENT for a missing file", async () => {
       const store = await register();
-      jest.spyOn(fs.promises, "stat").mockResolvedValue(undefined);
-      await expect(store.getAsStream("ghost.txt")).rejects.toThrow(
-        `File not found: ${path.join(tempDir, "ghost.txt")}`,
-      );
+      await expect(store.getAsStream("ghost.txt")).rejects.toMatchObject({
+        code: "ENOENT",
+      });
     });
   });
 
diff --git a/src/file-store.ts b/src/file-store.ts
index f08bd80..9f48012 100644
--- a/src/file-store.ts
+++ b/src/file-store.ts
@@ -79,7 +79,11 @@ const Configure = {
 const plugin: FastifyPluginAsync<{
   type: keyof typeof Configure;
 }> = async function (f, opts): Promise<void> {
-  const configure = Configure[opts.type];
+  // hasOwn, not a plain lookup: `Configure["toString"]` finds Object.prototype.toString
+  // and registers with no FileStore decorated, turning a typo into a silent no-op boot.
+  const configure = Object.prototype.hasOwnProperty.call(Configure, opts.type)
+    ? Configure[opts.type]
+    : undefined;
   if (!configure) {
     throw new Error(`Unknown storage type: ${opts.type}`);
   }
@@ -127,11 +131,10 @@ class LocalFileStore implements FileStore {
     await fs.promises.writeFile(p, data);
   }
   async getAsBuffer(filepath: string): Promise<Buffer> {
-    const p = path.join(this.dir, filepath);
-    if (await fs.promises.stat(p)) {
-      return fs.promises.readFile(p);
-    }
-    throw new Error(`File not found: ${p}`);
+    // No stat guard: a fulfilled stat is never falsy, so the old "File not found"
+    // branch was unreachable and readFile already rejects with ENOENT. Dropping it
+    // also removes a redundant stat and the TOCTOU window between the two calls.
+    return fs.promises.readFile(path.join(this.dir, filepath));
   }
   async copyFromLocalFile(
     filepath: string,
@@ -145,10 +148,10 @@ class LocalFileStore implements FileStore {
   }
   async getAsStream(filepath: string): Promise<NodeJS.ReadableStream> {
     const p = path.join(this.dir, filepath);
-    if (await fs.promises.stat(p)) {
-      return fs.createReadStream(p);
-    }
-    throw new Error(`File not found: ${p}`);
+    // stat first so a missing file rejects the promise rather than surfacing later as
+    // a stream 'error' — createReadStream is lazy, like the GCS reader.
+    await fs.promises.stat(p);
+    return fs.createReadStream(p);
   }
   async copyFromStream(
     filepath: string,
@@ -302,6 +305,14 @@ class GCPFileStore implements FileStore {
 
   async getAsStream(filepath: string): Promise<NodeJS.ReadableStream> {
     const gcsfile = this.storage.bucket(this.bucket).file(filepath);
+    // createReadStream does no I/O before returning, so without this check a missing
+    // object resolves and only fails later as a stream 'error' — which a caller doing
+    // `reply.send(await getAsStream(k))` turns into a 200 with a broken body, or an
+    // unhandled 'error' that takes the process down. Every other provider rejects.
+    const [exists] = await gcsfile.exists();
+    if (!exists) {
+      throw new Error(`File not found: ${filepath}`);
+    }
     return gcsfile.createReadStream();
   }
 
diff --git a/src/utils.spec.ts b/src/utils.spec.ts
index 9e40900..5462bf5 100644
--- a/src/utils.spec.ts
+++ b/src/utils.spec.ts
@@ -1,3 +1,4 @@
+import { EventEmitter } from "node:events";
 import { Readable } from "node:stream";
 import { DataStream, streamToBuffer } from "./utils";
 
@@ -163,15 +164,56 @@ describe("Utils", () => {
     });
   });
 
-  describe("DataStream interface", () => {
-    it("should define the correct interface structure", () => {
-      // This is a compile-time test to ensure the interface is properly defined
-      const mockStream: DataStream = {
-        on: jest.fn().mockReturnThis(),
-      };
+  describe("premature close", () => {
+    // A stream destroyed without an error emits only 'close'. Before this was handled
+    // the promise stayed pending forever and hung the request awaiting getAsBuffer.
+    it("rejects when the stream is destroyed without an error", async () => {
+      const stream = new Readable({ read() {} });
+      stream.push("partial");
+      setImmediate(() => stream.destroy());
+
+      await expect(streamToBuffer(stream)).rejects.toThrow(
+        "stream closed before end",
+      );
+    });
+
+    it("rejects with the original error when destroyed with one", async () => {
+      const stream = new Readable({ read() {} });
+      setImmediate(() => stream.destroy(new Error("boom")));
+
+      await expect(streamToBuffer(stream)).rejects.toThrow("boom");
+    });
+
+    it("still resolves normally when close follows end", async () => {
+      const stream: DataStream = Readable.from(["a", "b"]);
+
+      expect((await streamToBuffer(stream)).toString()).toBe("ab");
+    });
+
+    // Late events must not settle the promise a second time. EventEmitter is used
+    // directly so the exact sequence can be driven; a real Readable will not emit
+    // 'error' after 'end'.
+    it("ignores an error arriving after end", async () => {
+      const em = new EventEmitter() as unknown as DataStream;
+      const p = streamToBuffer(em);
+      const e = em as unknown as EventEmitter;
+      e.emit("data", Buffer.from("ok"));
+      e.emit("end");
+      e.emit("error", new Error("too late"));
+      e.emit("close");
+
+      expect((await p).toString()).toBe("ok");
+    });
+
+    it("ignores end and close arriving after an error", async () => {
+      const em = new EventEmitter() as unknown as DataStream;
+      const p = streamToBuffer(em);
+      const e = em as unknown as EventEmitter;
+      e.emit("error", new Error("first"));
+      e.emit("end");
+      e.emit("close");
 
-      expect(typeof mockStream.on).toBe("function");
-      expect(mockStream.on("test", () => {})).toBe(mockStream);
+      await expect(p).rejects.toThrow("first");
     });
   });
 });
diff --git a/src/utils.ts b/src/utils.ts
index 6468f0d..346b7a7 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -8,13 +8,29 @@ export interface DataStream {
 export function streamToBuffer(stream: DataStream): Promise<Buffer> {
   return new Promise<Buffer>((resolve, reject) => {
     const chunks: Buffer[] = [];
+    let settled = false;
+
     stream.on("data", (chunk: Buffer | string) =>
       chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)),
     );
     // Typed as Error so the rejection carries a stack; node streams always emit one.
-    stream.on("error", (err: Error) => reject(err));
-    stream.on("end", () =>
-      resolve(chunks.length === 1 ? chunks[0] : Buffer.concat(chunks)),
-    );
+    stream.on("error", (err: Error) => {
+      if (settled) return;
+      settled = true;
+      reject(err);
+    });
+    stream.on("end", () => {
+      if (settled) return;
+      settled = true;
+      resolve(chunks.length === 1 ? chunks[0] : Buffer.concat(chunks));
+    });
+    // A stream destroyed without an error argument emits only 'close', which would
+    // otherwise leave this promise pending forever and hang the awaiting request.
+    // 'close' follows 'end' in the normal path, where the settled flag ignores it.
+    stream.on("close", () => {
+      if (settled) return;
+      settled = true;
+      reject(new Error("stream closed before end"));
+    });
   });
 }

From 2b57363a5a2167abfdbaf3667015f3e51c1b048f Mon Sep 17 00:00:00 2001
From: Sabyasachi <saby@stackbox.xyz>
Date: Fri, 14 Aug 2026 12:18:04 +0000
Subject: [PATCH 2/3] test: assert write failures propagate, and run MinIO in
 CI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Every write test asserted only that a mock was called with the right arguments and
then resolved. Nothing fed a rejected write, so swallowing an error — dropping the
await, or attaching .catch(() => undefined) — left the suite green on all five
providers while writes silently failed. Six such mutations survived the previous
suite; two were re-run against this one and both now fail.

Adds a rejection test per write method across s3, gcs, azure and local, plus:

- Azure getContainerClient was never asserted, so every operation could target the
  wrong container unnoticed.
- Unicode payloads were untested — all fixtures were ASCII, so changing Azure's utf8
  encoding to ascii passed. Covered for azure, gcs and the MinIO round trip.
- The integration spec had no missing-key read. That is the real GetObject 404 path,
  the same error-shape class as the NoSuchKey/NotFound mismatch in #9, which mocks
  cannot catch.

CI now runs the integration suite against MinIO. It is started as a step rather than
a service container: services cannot pass a command and the minio image needs
"server /data". Skipping by default locally is right; never running in CI is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .github/workflows/test.yml               | 28 ++++++++++++++
 src/file-store.azure.spec.ts             | 49 +++++++++++++++++++++++-
 src/file-store.gcs.spec.ts               | 39 +++++++++++++++++++
 src/file-store.local.spec.ts             | 13 +++++++
 src/file-store.minio.integration.spec.ts | 20 ++++++++++
 src/file-store.s3.spec.ts                | 26 +++++++++++++
 6 files changed, 174 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index d4d3dfc..9329627 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -23,3 +23,31 @@ jobs:
       - run: pnpm run lint
       - run: pnpm run build
       - run: pnpm test
+
+      # Started as a step, not a service: `services:` cannot pass a command, and the
+      # minio image needs `server /data`. Without this the integration spec skips, and
+      # skip-by-default plus never-in-CI means it rots — which matters because mocked
+      # tests cannot catch an SDK behaviour change.
+      - name: Start MinIO
+        run: |
+          docker run -d --name ci-minio -p 9000:9000 \
+            -e MINIO_ROOT_USER=minioadmin \
+            -e MINIO_ROOT_PASSWORD=minioadmin \
+            minio/minio:latest server /data
+          for i in $(seq 1 30); do
+            if curl -sf http://127.0.0.1:9000/minio/health/live; then
+              echo "minio ready"; exit 0
+            fi
+            sleep 2
+          done
+          echo "minio did not become healthy" >&2
+          docker logs ci-minio >&2
+          exit 1
+
+      - run: pnpm run test:integration
+        env:
+          MINIO_TEST_ENDPOINT: http://127.0.0.1:9000
+
+      - name: Stop MinIO
+        if: always()
+        run: docker rm -f ci-minio || true
diff --git a/src/file-store.azure.spec.ts b/src/file-store.azure.spec.ts
index 873a79a..5667670 100644
--- a/src/file-store.azure.spec.ts
+++ b/src/file-store.azure.spec.ts
@@ -10,6 +10,7 @@ describe("AzureFileStore", () => {
   let fastify: ReturnType<typeof Fastify>;
   let mockBlobClient: any;
   let mockContainerClient: any;
+  let mockGetContainerClient: jest.Mock;
   let store: FileStore;
 
   beforeEach(async () => {
@@ -30,8 +31,9 @@ describe("AzureFileStore", () => {
       getBlockBlobClient: jest.fn(() => mockBlobClient),
     };
 
+    mockGetContainerClient = jest.fn(() => mockContainerClient);
     (BlobServiceClient as unknown as jest.Mock).mockImplementation(() => ({
-      getContainerClient: jest.fn(() => mockContainerClient),
+      getContainerClient: mockGetContainerClient,
     }));
 
     process.env.AZURE_STORAGE_ACCOUNT_URL =
@@ -58,6 +60,12 @@ describe("AzureFileStore", () => {
       expect(store).toBeDefined();
     });
 
+    // Nothing asserted the container name, so every operation could target the wrong
+    // container and the suite would stay green.
+    it("opens the container named by AZURE_STORAGE_CONTAINER", () => {
+      expect(mockGetContainerClient).toHaveBeenCalledWith("test-container");
+    });
+
     it("should throw when AZURE_STORAGE_ACCOUNT_URL is missing", async () => {
       delete process.env.AZURE_STORAGE_ACCOUNT_URL;
       const f = Fastify();
@@ -102,6 +110,27 @@ describe("AzureFileStore", () => {
       );
     });
 
+    // Without this, changing the utf8 encoding to ascii mangles every non-ASCII byte
+    // and the suite stays green — all other payload fixtures are ASCII.
+    it("encodes a unicode payload as utf8", async () => {
+      mockBlobClient.uploadData.mockResolvedValueOnce({});
+
+      await store.save("u.txt", "text/plain", "héllo→世界");
+
+      expect(mockBlobClient.uploadData).toHaveBeenCalledWith(
+        Buffer.from("héllo→世界", "utf8"),
+        { blobHTTPHeaders: { blobContentType: "text/plain" } },
+      );
+    });
+
+    it("propagates a rejected upload, not just an errorCode", async () => {
+      mockBlobClient.uploadData.mockRejectedValueOnce(new Error("AuthFailure"));
+
+      await expect(store.save("a.txt", "text/plain", "x")).rejects.toThrow(
+        "AuthFailure",
+      );
+    });
+
     it("should pass a Buffer payload through untouched", async () => {
       mockBlobClient.uploadData.mockResolvedValueOnce({});
       const data = Buffer.from([1, 2, 3]);
@@ -140,6 +169,24 @@ describe("AzureFileStore", () => {
     });
   });
 
+  describe("write failures", () => {
+    it("propagates a rejected uploadFile", async () => {
+      mockBlobClient.uploadFile.mockRejectedValueOnce(new Error("io error"));
+
+      await expect(
+        store.copyFromLocalFile("a.txt", "text/plain", "/tmp/x"),
+      ).rejects.toThrow("io error");
+    });
+
+    it("propagates a rejected uploadStream", async () => {
+      mockBlobClient.uploadStream.mockRejectedValueOnce(new Error("aborted"));
+
+      await expect(
+        store.copyFromStream("a.txt", "text/plain", Readable.from(["x"])),
+      ).rejects.toThrow("aborted");
+    });
+  });
+
   describe("copyFromLocalFile", () => {
     it("should upload the local file", async () => {
       mockBlobClient.uploadFile.mockResolvedValueOnce({});
diff --git a/src/file-store.gcs.spec.ts b/src/file-store.gcs.spec.ts
index 7a48193..7af6b1f 100644
--- a/src/file-store.gcs.spec.ts
+++ b/src/file-store.gcs.spec.ts
@@ -88,6 +88,24 @@ describe("GCPFileStore", () => {
         contentType: "text/plain",
       });
     });
+
+    it("propagates a failed save", async () => {
+      mockFile.save.mockRejectedValueOnce(new Error("quota exceeded"));
+
+      await expect(
+        store.save("test.txt", "text/plain", "content"),
+      ).rejects.toThrow("quota exceeded");
+    });
+
+    it("round-trips a unicode payload unchanged", async () => {
+      mockFile.save.mockResolvedValueOnce(undefined);
+
+      await store.save("u.txt", "text/plain", "héllo→世界");
+
+      expect(mockFile.save).toHaveBeenCalledWith("héllo→世界", {
+        contentType: "text/plain",
+      });
+    });
   });
 
   describe("getAsBuffer", () => {
@@ -113,6 +131,14 @@ describe("GCPFileStore", () => {
         destination: "dest.txt",
       });
     });
+
+    it("propagates a failed upload", async () => {
+      mockBucket.upload.mockRejectedValueOnce(new Error("upload failed"));
+
+      await expect(
+        store.copyFromLocalFile("dest.txt", "text/plain", "/tmp/src.txt"),
+      ).rejects.toThrow("upload failed");
+    });
   });
 
   describe("getAsStream", () => {
@@ -159,6 +185,19 @@ describe("GCPFileStore", () => {
       });
       expect(Buffer.concat(chunks).toString()).toBe("piped content");
     });
+
+    it("propagates a write-stream failure", async () => {
+      const ws = new Writable({
+        write(_chunk, _enc, cb) {
+          cb(new Error("disk full"));
+        },
+      });
+      mockFile.createWriteStream.mockReturnValueOnce(ws);
+
+      await expect(
+        store.copyFromStream("test.txt", "text/plain", Readable.from(["x"])),
+      ).rejects.toThrow("disk full");
+    });
   });
 
   describe("getInfo", () => {
diff --git a/src/file-store.local.spec.ts b/src/file-store.local.spec.ts
index afec7ac..7bf0318 100644
--- a/src/file-store.local.spec.ts
+++ b/src/file-store.local.spec.ts
@@ -227,6 +227,19 @@ describe("LocalFileStore", () => {
         await fs.promises.readFile(path.join(tempDir, "x/y/z.txt"), "utf8"),
       ).toBe("piped");
     });
+
+    it("propagates a source stream error", async () => {
+      const store = await register();
+      const bad = new Readable({
+        read() {
+          this.destroy(new Error("source failed"));
+        },
+      });
+
+      await expect(
+        store.copyFromStream("bad.txt", "text/plain", bad),
+      ).rejects.toThrow("source failed");
+    });
   });
 });
 
diff --git a/src/file-store.minio.integration.spec.ts b/src/file-store.minio.integration.spec.ts
index bd05aa8..2f86c0a 100644
--- a/src/file-store.minio.integration.spec.ts
+++ b/src/file-store.minio.integration.spec.ts
@@ -64,6 +64,26 @@ describeIf("MinIO integration", () => {
     expect(await store.getInfo(`missing-${Date.now()}.txt`)).toBeNull();
   });
 
+  // The real GetObject 404 path. Mocks cannot catch a change in this error shape —
+  // the NoSuchKey/NotFound mismatch fixed in #9 was invisible to them.
+  it("rejects getAsBuffer for a missing key", async () => {
+    await expect(
+      store.getAsBuffer(`missing-${Date.now()}.txt`),
+    ).rejects.toBeDefined();
+  });
+
+  it("rejects getAsStream for a missing key", async () => {
+    await expect(
+      store.getAsStream(`missing-${Date.now()}.txt`),
+    ).rejects.toBeDefined();
+  });
+
+  it("round-trips a unicode payload and key", async () => {
+    const key = `unicode-héllo-世界-${Date.now()}.txt`;
+    await store.save(key, "text/plain", "héllo→世界");
+    expect((await store.getAsBuffer(key)).toString()).toBe("héllo→世界");
+  });
+
   it("round-trips a string payload", async () => {
     const key = `round-trip-${Date.now()}.txt`;
     await store.save(key, "text/plain", "hello minio");
diff --git a/src/file-store.s3.spec.ts b/src/file-store.s3.spec.ts
index c5fcb2b..598a19d 100644
--- a/src/file-store.s3.spec.ts
+++ b/src/file-store.s3.spec.ts
@@ -208,6 +208,16 @@ describe("S3FileStore", () => {
         });
         expect(mockSend).toHaveBeenCalledWith(expect.any(S3.PutObjectCommand));
       });
+
+      // Without this, deleting the await or attaching .catch(() => undefined) leaves
+      // the suite green while every write silently fails.
+      it("propagates a failed PutObject", async () => {
+        mockSend.mockRejectedValueOnce(new Error("AccessDenied"));
+
+        await expect(
+          store.save("a/b.txt", "text/plain", "content"),
+        ).rejects.toThrow("AccessDenied");
+      });
     });
 
     describe("getAsBuffer", () => {
@@ -264,6 +274,14 @@ describe("S3FileStore", () => {
 
         params.params.Body.destroy();
       });
+
+      it("propagates a failed upload", async () => {
+        mockDone.mockRejectedValueOnce(new Error("upload aborted"));
+
+        await expect(
+          store.copyFromLocalFile("a/b.ts", "text/plain", __filename),
+        ).rejects.toThrow("upload aborted");
+      });
     });
 
     describe("copyFromStream", () => {
@@ -283,6 +301,14 @@ describe("S3FileStore", () => {
         });
         expect(mockDone).toHaveBeenCalled();
       });
+
+      it("propagates a failed upload", async () => {
+        mockDone.mockRejectedValueOnce(new Error("premature close"));
+
+        await expect(
+          store.copyFromStream("a/b.txt", "text/plain", Readable.from(["x"])),
+        ).rejects.toThrow("premature close");
+      });
     });
 
     describe("getInfo", () => {

From 1411ce9f1fa812adcc896cc12b56d4276491476d Mon Sep 17 00:00:00 2001
From: Sabyasachi <saby@stackbox.xyz>
Date: Fri, 14 Aug 2026 12:11:59 +0000
Subject: [PATCH 3/3] ci(release): publish on release published, and verify tag
 matches version

The trigger was release:created. GitHub fires created when a draft release is saved,
which would publish from the default branch before the tag exists; it then fires
published (not created) when that draft is released, so the real release would ship
nothing. Every release so far was created and published in one action, which is why
this never bit.

The job also published whatever version happened to be in package.json at the release
commit, so a tag and a version could disagree silently. A guard now compares them and
fails the job.

publishConfig.registry pins GitHub Packages. CI worked only because setup-node writes
the runner .npmrc; a local npm publish would target npmjs.org and fail against a scope
we do not own.

Lint and build join the pre-publish test gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../workflows/npm-publish-github-packages.yml | 28 ++++++++++++++++---
 package.json                                  |  5 +++-
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/npm-publish-github-packages.yml b/.github/workflows/npm-publish-github-packages.yml
index e8d96a3..5faba15 100644
--- a/.github/workflows/npm-publish-github-packages.yml
+++ b/.github/workflows/npm-publish-github-packages.yml
@@ -1,11 +1,15 @@
-# This workflow will run tests using node and then publish a package to GitHub Packages when a release is created
-# For more information see: https://help.github.com/actions/language-and-framework-guides/publishing-nodejs-packages
+# Publishes to GitHub Packages when a release is published.
+# See .claude/skills/release/SKILL.md for the release procedure.
 
 name: Node.js Package
 
 on:
   release:
-    types: [created]
+    # published, not created: GitHub fires `created` when a *draft* is saved, which
+    # would publish from the default branch before the tag exists — and then fires
+    # `published` (not `created`) when that draft is released, so the real release
+    # would ship nothing.
+    types: [published]
 
 env:
   NODE_VERSION: 24
@@ -21,6 +25,8 @@ jobs:
           node-version: ${{ env.NODE_VERSION }}
           cache: pnpm
       - run: pnpm install --frozen-lockfile
+      - run: pnpm run lint
+      - run: pnpm run build
       - run: pnpm test
 
   publish-gpr:
@@ -38,9 +44,23 @@ jobs:
           cache: pnpm
           registry-url: https://npm.pkg.github.com/
       - run: pnpm install --frozen-lockfile
+
+      # The release tag and package.json must agree, or the published version silently
+      # differs from the tag people will go looking for. TAG comes through env rather
+      # than being interpolated into the script.
+      - name: Verify tag matches package.json version
+        env:
+          TAG: ${{ github.event.release.tag_name }}
+        run: |
+          VERSION="v$(node -p 'require("./package.json").version')"
+          if [ "$VERSION" != "$TAG" ]; then
+            echo "Release tag $TAG does not match package.json $VERSION" >&2
+            exit 1
+          fi
+
       # npm publish, not pnpm publish: pnpm adds git-state checks the release flow
       # does not need. prepublishOnly runs the build either way.
       # No --provenance: GitHub Packages does not accept provenance attestations.
       - run: npm publish
         env:
-          NODE_AUTH_TOKEN: ${{secrets.GITHUB_TOKEN}}
+          NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/package.json b/package.json
index b2d3cf6..74e5350 100644
--- a/package.json
+++ b/package.json
@@ -63,5 +63,8 @@
     "typescript": "^6.0.3",
     "typescript-eslint": "^8.67.0"
   },
-  "packageManager": "pnpm@10.13.1"
+  "packageManager": "pnpm@10.13.1",
+  "publishConfig": {
+    "registry": "https://npm.pkg.github.com/"
+  }
 }