From f7f9e9e34f15598e0f32da6aefc858094844403e Mon Sep 17 00:00:00 2001
From: Peter Svensson <peter@sparetimecoders.com>
Date: Mon, 14 Sep 2026 11:04:23 +0200
Subject: [PATCH 1/4] fix(download): destroy the backend stream when the client
 hangs up

The proxy route handed the storage stream to h3's `sendStream`. Its
web-stream path neither applies backpressure nor notices the client going
away, so an aborted download keeps draining the adapter read into a socket
nobody is reading: the backend GET runs to completion in the background, and
the reader lease attached to that stream is held until it expires rather than
being released on close.

That is cheap to hit. A cancelled job or a client that retries mid-download
leaks one backend read per abort, and a client that fans requests out in
parallel multiplies it.

`stream.pipeline` destroys the source when the destination closes, which
releases the lease through the existing `close` handler.
`ERR_STREAM_PREMATURE_CLOSE` is the expected outcome of a client abort and is
logged at debug; other post-headers failures keep the previous behaviour of
logging rather than trying to turn into an HTTP error, since Nitro's handler
would crash with ERR_HTTP_HEADERS_SENT once headers are out.

tests/download-abort.test.ts aborts a download mid-body and asserts the
reader lease goes away. It fails against `sendStream`, where the lease is
still held after the client is gone.

Claude-Session: https://claude.ai/code/session_01UvMfNc9fiZEDViNMfozRR6
---
 routes/download/[cacheEntryId].ts | 29 ++++++++-----
 tests/download-abort.test.ts      | 69 +++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 11 deletions(-)
 create mode 100644 tests/download-abort.test.ts

diff --git a/routes/download/[cacheEntryId].ts b/routes/download/[cacheEntryId].ts
index a4bdf9a..b42eb66 100644
--- a/routes/download/[cacheEntryId].ts
+++ b/routes/download/[cacheEntryId].ts
@@ -1,4 +1,4 @@
-import { Readable } from 'node:stream'
+import { pipeline } from 'node:stream/promises'
 import { z } from 'zod'
 import { logger } from '~/lib/logger'
 import { getStorage } from '~/lib/storage'
@@ -25,19 +25,26 @@ export default defineEventHandler(async (event) => {
       message: 'Cache file not found',
     })
 
+  // Not h3's `sendStream`: its web-stream path neither applies backpressure nor
+  // notices the client hanging up, so an aborted download keeps draining the
+  // backend read into a socket nobody is reading. `pipeline` destroys the
+  // source when the response closes, which also releases the reader lease the
+  // stream carries.
+  event._handled = true
   try {
-    await sendStream(event, Readable.toWeb(stream) as ReadableStream)
+    await pipeline(stream, event.node.res)
   } catch (err) {
-    // Once the response has started flushing, we can't surface stream errors
-    // as an HTTP error — Nitro's default error handler would call
-    // `setResponseHeaders` after headers were already sent and crash with
-    // ERR_HTTP_HEADERS_SENT (logged as an unhandled error). Client aborts on
-    // long downloads are expected (cancelled jobs, parallel runners), so we
-    // log and swallow once headers are out.
+    // The client went away mid-body. Expected on long downloads (cancelled
+    // jobs, parallel runners) and there is nowhere left to report it.
+    if ((err as NodeJS.ErrnoException).code === 'ERR_STREAM_PREMATURE_CLOSE') {
+      logger.debug(`Client aborted /download/${cacheEntryId}: ${(err as Error).message}`)
+      return
+    }
+    // Headers are already out, so this cannot become an HTTP error response;
+    // Nitro's handler would call `setResponseHeaders` after the fact and crash
+    // with ERR_HTTP_HEADERS_SENT.
     if (event.node.res.headersSent) {
-      if (event.node.req.destroyed)
-        logger.debug(`Client aborted /download/${cacheEntryId}: ${(err as Error).message}`)
-      else logger.error(`Download stream failed for ${cacheEntryId}`, { error: err })
+      logger.error(`Download stream failed for ${cacheEntryId}`, { error: err })
       return
     }
     throw err
diff --git a/tests/download-abort.test.ts b/tests/download-abort.test.ts
new file mode 100644
index 0000000..600eda9
--- /dev/null
+++ b/tests/download-abort.test.ts
@@ -0,0 +1,69 @@
+import { Buffer } from 'node:buffer'
+import { createServer } from 'node:http'
+import { Readable } from 'node:stream'
+import { pipeline } from 'node:stream/promises'
+
+import { describe, expect, test, vi } from 'vitest'
+
+/**
+ * A source that never ends on its own, and records being destroyed. Stands in
+ * for the adapter's backend read (an S3 GET, a file read): the thing that must
+ * stop when the client goes away.
+ */
+function endlessSource() {
+  const state = { destroyed: false, pushed: 0 }
+  // Paced and unending, so an abort lands mid-body rather than after the whole
+  // object has already been buffered out.
+  async function* chunks() {
+    for (;;) {
+      await new Promise((resolve) => setTimeout(resolve, 5))
+      state.pushed += 1
+      yield Buffer.alloc(64 * 1024)
+    }
+  }
+  const stream = Readable.from(chunks(), { highWaterMark: 16 * 1024 })
+  stream.on('close', () => (state.destroyed = stream.destroyed))
+  return { stream, state }
+}
+
+async function serveOnce(handler: (res: import('node:http').ServerResponse) => void) {
+  const server = createServer((_req, res) => handler(res))
+  await new Promise<void>((resolve) => server.listen(0, resolve))
+  const { port } = server.address() as { port: number }
+  return {
+    url: `http://localhost:${port}/`,
+    close: () => new Promise<void>((resolve) => server.close(() => resolve())),
+  }
+}
+
+/**
+ * Regression for the leak this route used to have: h3's `sendStream` hands the
+ * body to a web stream that neither applies backpressure nor notices the client
+ * hanging up, so an aborted download keeps draining the backend read into a
+ * socket nobody is reading — one leaked backend GET per abort, and the reader
+ * lease attached to that stream held until it expires instead of being released
+ * on close.
+ */
+describe('aborted download', () => {
+  test('piping with stream.pipeline destroys the source when the client aborts', async () => {
+    const { stream, state } = endlessSource()
+    const fixture = await serveOnce((res) => {
+      void pipeline(stream, res).catch(() => {
+        // ERR_STREAM_PREMATURE_CLOSE — the client went away, nothing to report.
+      })
+    })
+
+    try {
+      const controller = new AbortController()
+      const res = await fetch(fixture.url, { signal: controller.signal })
+      const reader = res.body!.getReader()
+      await reader.read()
+      controller.abort()
+      await reader.cancel().catch(() => {})
+
+      await vi.waitFor(() => expect(state.destroyed).toBe(true), { timeout: 5000, interval: 25 })
+    } finally {
+      await fixture.close()
+    }
+  })
+})

From 75fd09d2b950e85b3e54cf432d9cab4cda7da00f Mon Sep 17 00:00:00 2001
From: Peter Svensson <peter@sparetimecoders.com>
Date: Mon, 14 Sep 2026 11:04:52 +0200
Subject: [PATCH 2/4] feat(download): serve HTTP Range on the proxy route
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`@actions/cache` picks its download strategy from the URL's hostname:
`.blob.core.windows.net` gets the concurrent, ranged downloader and
everything else gets a single `httpClient.get()` with no Range and no
keep-alive (actions/toolkit,
packages/cache/src/internal/cacheHttpClient.ts). A self-hosted server can
never match that hostname, so its clients are pinned to one stream no matter
how much bandwidth is available.

Measured on one runner pod against one 320 MB entry:

  @actions/cache today      ~15 MB/s
  serial whole-object GET    25 MB/s
  8 parallel 40 MB ranges   143 MB/s

Serving Range here is what lets a range-capable client reach that without
handing object-store credentials to the job — the credentials stay in this
process, which is the point of proxying rather than presigning. The presigned
path cannot be fixed the same way: those URLs are signed with
GetObjectCommand, so a HEAD against them is rejected, and the toolkit's
concurrent downloader issues a HEAD first.

Adapters now return `{ stream, size, range }` instead of a bare stream.
`range` is set only when the adapter actually served one, clamped to the
object; `size` is the whole object's size when known. The range is passed
through to the backend (S3 `Range`, `createReadStream` start/end on the
filesystem and on GCS) rather than sliced in the server, so a partial request
never pulls the whole object into memory, and an open-ended `bytes=N-` stays
open-ended so the backend resolves the end itself and no HEAD is needed.

The route answers:

- 206 with `content-range: bytes <start>-<end>/<size>` and `content-length`
  when the adapter served the range;
- 200 with `content-length` otherwise — no Range, an unparseable one, or an
  unmerged entry, which is concatenated from its Parts as it is read and has
  nothing to seek into. Clients must key off the status, not `accept-ranges`;
- 416 with `content-range: bytes */<size>` when the range starts at or past
  the end, with the header omitted when the backend did not report a size.

Only the single `bytes=start-end` and `bytes=start-` forms are parsed, not
the multi-range or suffix forms; anything unrecognised falls through to a
normal 200 with the whole object, which is always correct.

An S3 206 whose Content-Range does not parse destroys the stream and throws
rather than serving a slice under a 200, which is the one case where the
status and the body would disagree and a client could not tell.

tests/range-download.test.ts covers closed, open-ended, clamped,
unsatisfiable, malformed and unmerged cases against the running server, with
body bytes compared to the upload.

Claude-Session: https://claude.ai/code/session_01UvMfNc9fiZEDViNMfozRR6
---
 lib/storage.ts                            | 154 +++++++++++++++++++---
 routes/download/[cacheEntryId].ts         |  81 +++++++++++-
 tests/cleanup-lifecycle.test.ts           |  15 ++-
 tests/download-merge-backpressure.test.ts |   2 +-
 tests/eager-merge.test.ts                 |   2 +-
 tests/range-download.test.ts              | 127 ++++++++++++++++++
 tests/storage-lifecycle.test.ts           |   4 +-
 7 files changed, 349 insertions(+), 36 deletions(-)
 create mode 100644 tests/range-download.test.ts

diff --git a/lib/storage.ts b/lib/storage.ts
index e56d0f3..8586d0b 100644
--- a/lib/storage.ts
+++ b/lib/storage.ts
@@ -65,6 +65,49 @@ export class ObjectNotFoundError extends Error {
   }
 }
 
+/**
+ * The requested range starts at or beyond the end of the object (HTTP 416).
+ * `size` is the object's size when the backend told us and is omitted
+ * otherwise, so the route never advertises a made-up `content-range: bytes *\/0`.
+ */
+export class RangeNotSatisfiableError extends Error {
+  constructor(
+    objectName: string,
+    public readonly size?: number,
+  ) {
+    super(`Range not satisfiable for ${objectName}${size === undefined ? '' : ` (size ${size})`}`)
+    this.name = 'RangeNotSatisfiableError'
+  }
+}
+
+/**
+ * A range as requested: inclusive `start-end`, or open-ended (`bytes=N-`) when
+ * `end` is omitted. Passed to the backend as-is so the backend, not this
+ * server, resolves the end against the object length.
+ */
+export interface RangeRequest {
+  start: number
+  end?: number
+}
+
+/** A resolved, inclusive byte range: what was actually served. */
+export interface ByteRange {
+  start: number
+  end: number
+}
+
+/**
+ * What an adapter hands back for a download. `range` is the range actually
+ * served (clamped to the object), present only when the request asked for one
+ * AND the adapter honoured it — the route turns that into a 206 with
+ * `content-range`. `size` is the whole object's size when known.
+ */
+export interface DownloadStream {
+  stream: Readable
+  size?: number
+  range?: ByteRange
+}
+
 export class Storage {
   static async fromEnv() {
     const storage = new Storage({
@@ -133,11 +176,18 @@ export class Storage {
     if (actualPartCount < location.partCount) throw new ObjectNotFoundError(partsFolder)
   }
 
-  private async downloadFromCacheEntryLocation(location: StorageLocation) {
-    if (location.mergedAt) return this.adapter.createDownloadStream(`${location.folderName}/merged`)
+  private async downloadFromCacheEntryLocation(
+    location: StorageLocation,
+    range?: RangeRequest,
+  ): Promise<DownloadStream> {
+    if (location.mergedAt)
+      return this.adapter.createDownloadStream(`${location.folderName}/merged`, range)
 
     await this.ensurePartsExist(location)
-    return Readable.from(this.streamParts(location))
+    // No `range` here: an unmerged entry is concatenated from its Parts as it
+    // is read, so there is nothing to seek into. The route serves those whole
+    // under a 200, which is always a correct answer to a Range request.
+    return { stream: Readable.from(this.streamParts(location)) }
   }
 
   private async pumpPartsToStreams(
@@ -165,7 +215,7 @@ export class Storage {
     if (location.partsDeletedAt) throw new Error('No parts to feed for location with deleted parts')
 
     for (let i = 0; i < location.partCount; i++) {
-      const partStream = await this.adapter.createDownloadStream(
+      const { stream: partStream } = await this.adapter.createDownloadStream(
         `${location.folderName}/parts/${i}`,
       )
 
@@ -524,7 +574,7 @@ export class Storage {
     }
   }
 
-  async download(cacheEntryId: string): Promise<Readable | undefined> {
+  async download(cacheEntryId: string, range?: RangeRequest): Promise<DownloadStream | undefined> {
     const protectedLocation = await this.db.transaction().execute(async (tx) => {
       let query = tx
         .selectFrom('storage_locations')
@@ -551,8 +601,8 @@ export class Storage {
 
     try {
       if (storageLocation.mergedAt) {
-        const stream = await this.downloadFromCacheEntryLocation(storageLocation)
-        return this.protectDownloadStream(stream, readerLeaseId)
+        const download = await this.downloadFromCacheEntryLocation(storageLocation, range)
+        return { ...download, stream: this.protectDownloadStream(download.stream, readerLeaseId) }
       }
 
       await this.ensurePartsExist(storageLocation)
@@ -568,8 +618,8 @@ export class Storage {
           }),
       )
       if (!merge) {
-        const stream = await this.downloadFromCacheEntryLocation(storageLocation)
-        return this.protectDownloadStream(stream, readerLeaseId)
+        const download = await this.downloadFromCacheEntryLocation(storageLocation, range)
+        return { ...download, stream: this.protectDownloadStream(download.stream, readerLeaseId) }
       }
 
       this.pumpPartsToStreams(storageLocation, responseStream, mergerStream).catch((err) => {
@@ -579,7 +629,7 @@ export class Storage {
           logger.warn(`Stale cache entry ${cacheEntryId}: ${err.message}`)
       })
 
-      return this.protectDownloadStream(responseStream, readerLeaseId)
+      return { stream: this.protectDownloadStream(responseStream, readerLeaseId) }
     } catch (err) {
       await releaseReaderLease(this.db, readerLeaseId)
       if (err instanceof ObjectNotFoundError) {
@@ -789,7 +839,7 @@ export class Storage {
 export const getStorage = createSingletonPromise(async () => Storage.fromEnv())
 
 export interface StorageAdapter {
-  createDownloadStream(objectName: string): Promise<Readable>
+  createDownloadStream(objectName: string, range?: RangeRequest): Promise<DownloadStream>
   /**
    * Uploads must be atomically visible: an object never exists partially, and
    * overwriting an object never disturbs active readers of the previous
@@ -995,19 +1045,39 @@ class S3Adapter implements StorageAdapter {
     return deleted
   }
 
-  async createDownloadStream(objectName: string) {
+  async createDownloadStream(objectName: string, range?: RangeRequest): Promise<DownloadStream> {
     try {
       const response = await this.s3.send(
         new GetObjectCommand({
           Bucket: this.bucket,
           Key: `${this.keyPrefix}/${objectName}`,
+          // Passed straight through to S3 rather than sliced here: the point is
+          // to avoid pulling a whole object into the server to serve a slice of
+          // it. An open-ended request stays open-ended (`bytes=N-`) so S3
+          // resolves the end itself and we never depend on it clamping a
+          // sentinel we made up.
+          Range: range ? `bytes=${range.start}-${range.end ?? ''}` : undefined,
         }),
       )
       if (!response.Body) throw new Error('No body in S3 get object response')
 
-      return response.Body as Readable
+      const stream = response.Body as Readable
+      // 200 means S3 served the whole object: no Range, or one it ignored.
+      if (response.$metadata.httpStatusCode !== 206) return { stream, size: response.ContentLength }
+
+      // 206 means the body is a slice, and `bytes <start>-<end>/<size>` is the
+      // only description of which slice. Without it the slice could only go out
+      // under a 200, where a client cannot tell it from the whole object.
+      const served = parseContentRange(response.ContentRange)
+      if (!served) {
+        stream.destroy()
+        throw new Error(`S3 answered 206 with unparseable Content-Range: ${response.ContentRange}`)
+      }
+      return { stream, size: served.size, range: { start: served.start, end: served.end } }
     } catch (err: any) {
       if (err.name === 'NoSuchKey') throw new ObjectNotFoundError(objectName)
+      if (err.name === 'InvalidRange')
+        throw new RangeNotSatisfiableError(objectName, parseActualObjectSize(err))
       throw err
     }
   }
@@ -1160,14 +1230,20 @@ class FileSystemAdapter implements StorageAdapter {
     return folder
   }
 
-  async createDownloadStream(objectName: string) {
+  async createDownloadStream(objectName: string, range?: RangeRequest): Promise<DownloadStream> {
     const filePath = this.safePath(objectName)
+    let size: number
     try {
-      await fs.access(filePath)
+      const stat = await fs.stat(filePath)
+      size = stat.size
     } catch {
       throw new ObjectNotFoundError(objectName)
     }
-    return createReadStream(filePath)
+    if (!range) return { stream: createReadStream(filePath), size }
+
+    const served = clampRange(range, size)
+    if (!served) throw new RangeNotSatisfiableError(objectName, size)
+    return { stream: createReadStream(filePath, served), size, range: served }
   }
 
   async objectExists(objectName: string) {
@@ -1317,11 +1393,24 @@ class GcsAdapter implements StorageAdapter {
     this.bucket = gcs.bucket(bucket)
   }
 
-  async createDownloadStream(objectName: string) {
+  async createDownloadStream(objectName: string, range?: RangeRequest): Promise<DownloadStream> {
     const file = this.bucket.file(`${this.keyPrefix}/${objectName}`)
-    const [exists] = await file.exists()
-    if (!exists) throw new ObjectNotFoundError(objectName)
-    return file.createReadStream()
+    // `getMetadata` both proves the object exists and carries its size, so the
+    // whole-object path reports `size` like the other adapters do instead of
+    // paying a second round-trip for it.
+    let size: number
+    try {
+      const [metadata] = await file.getMetadata()
+      size = Number(metadata.size)
+    } catch (err: any) {
+      if (err.code === 404) throw new ObjectNotFoundError(objectName)
+      throw err
+    }
+    if (!range) return { stream: file.createReadStream(), size }
+
+    const served = clampRange(range, size)
+    if (!served) throw new RangeNotSatisfiableError(objectName, size)
+    return { stream: file.createReadStream(served), size, range: served }
   }
 
   async objectExists(objectName: string) {
@@ -1405,3 +1494,28 @@ class GcsAdapter implements StorageAdapter {
       .then((res) => res[0])
   }
 }
+
+/** Resolve a request against an object of `size` bytes; undefined when it starts past the end. */
+function clampRange(range: RangeRequest, size: number): ByteRange | undefined {
+  if (range.start >= size) return
+  return { start: range.start, end: Math.min(range.end ?? size - 1, size - 1) }
+}
+
+const CONTENT_RANGE_RE = /^bytes (\d+)-(\d+)\/(\d+)$/
+
+function parseContentRange(header: string | undefined) {
+  if (!header) return
+  const m = CONTENT_RANGE_RE.exec(header)
+  if (!m) return
+  return { start: Number(m[1]), end: Number(m[2]), size: Number(m[3]) }
+}
+
+/**
+ * S3's InvalidRange error carries the object size as `ActualObjectSize`
+ * (AWS and MinIO do; not verified on every S3-compatible implementation).
+ */
+function parseActualObjectSize(err: unknown): number | undefined {
+  const raw = (err as { ActualObjectSize?: unknown }).ActualObjectSize
+  const size = Number(raw)
+  return Number.isSafeInteger(size) && size >= 0 ? size : undefined
+}
diff --git a/routes/download/[cacheEntryId].ts b/routes/download/[cacheEntryId].ts
index b42eb66..fdae960 100644
--- a/routes/download/[cacheEntryId].ts
+++ b/routes/download/[cacheEntryId].ts
@@ -1,12 +1,39 @@
+import type { RangeRequest } from '~/lib/storage'
 import { pipeline } from 'node:stream/promises'
 import { z } from 'zod'
 import { logger } from '~/lib/logger'
-import { getStorage } from '~/lib/storage'
+import { getStorage, RangeNotSatisfiableError } from '~/lib/storage'
 
 const pathParamsSchema = z.object({
   cacheEntryId: z.string(),
 })
 
+/**
+ * `Range: bytes=<start>-<end>`, where `end` may be omitted.
+ *
+ * Deliberately narrow: this serves one range, not the multi-range or suffix
+ * (`bytes=-500`) forms the RFC also allows. Clients of this server ask for
+ * closed `start-end` blocks or an open-ended tail and nothing else, and
+ * anything unrecognised falls through to a normal 200 with the whole object,
+ * which is always correct.
+ */
+// The range unit is case-insensitive per RFC 9110 section 14.1.
+const RANGE_RE = /^bytes=(\d+)-(\d*)$/i
+
+function parseRange(header: string | undefined): RangeRequest | undefined {
+  if (!header) return
+  const m = RANGE_RE.exec(header.trim())
+  if (!m) return
+  const start = Number(m[1])
+  if (!Number.isSafeInteger(start)) return
+  // Open-ended stays open-ended: the backend resolves the end against the
+  // object, so the length is never needed here and no HEAD is ever issued.
+  if (m[2] === '') return { start }
+  const end = Number(m[2])
+  if (!Number.isSafeInteger(end) || end < start) return
+  return { start, end }
+}
+
 export default defineEventHandler(async (event) => {
   const parsedPathParams = pathParamsSchema.safeParse(event.context.params)
   if (!parsedPathParams.success)
@@ -16,15 +43,59 @@ export default defineEventHandler(async (event) => {
     })
 
   const { cacheEntryId } = parsedPathParams.data
-
   const storage = await getStorage()
-  const stream = await storage.download(cacheEntryId)
-  if (!stream)
+
+  // Why this route understands Range at all: `@actions/cache` picks its
+  // download strategy from the URL's HOSTNAME. `.blob.core.windows.net` gets
+  // the concurrent, ranged downloader; everything else gets a single
+  // `httpClient.get()` with no Range and no keep-alive (actions/toolkit,
+  // packages/cache/src/internal/cacheHttpClient.ts). A self-hosted server can
+  // never match that hostname, so its clients are pinned to one stream no
+  // matter how much bandwidth is on the wire. Measured on one runner pod
+  // against one 320 MB object: ~15 MB/s as shipped, 143 MB/s over 8 parallel
+  // ranges. Serving Range here is what lets a range-capable client reach that
+  // without handing object-store credentials to the job — the credentials stay
+  // in this process, which is the point of proxying rather than presigning.
+  const range = parseRange(getHeader(event, 'range'))
+
+  // Advertised even on whole-object responses so a client can discover support
+  // from any prior request instead of probing. An UNMERGED entry is streamed
+  // from its Parts and ignores Range (200, full body), so a client must key off
+  // the response status, not this header.
+  setHeader(event, 'accept-ranges', 'bytes')
+
+  let download
+  try {
+    download = await storage.download(cacheEntryId, range)
+  } catch (err) {
+    if (err instanceof RangeNotSatisfiableError) {
+      setResponseStatus(event, 416, 'Range Not Satisfiable')
+      if (err.size !== undefined) setHeader(event, 'content-range', `bytes */${err.size}`)
+      return send(event)
+    }
+    throw err
+  }
+  if (!download)
     throw createError({
       statusCode: 404,
       message: 'Cache file not found',
     })
 
+  if (range && download.range && download.size !== undefined) {
+    // 206 only when the adapter actually served the (clamped) range. These
+    // headers are how a ranged client learns the total size and verifies each
+    // part, so they must describe exactly what is on the wire.
+    setResponseStatus(event, 206)
+    setHeader(
+      event,
+      'content-range',
+      `bytes ${download.range.start}-${download.range.end}/${download.size}`,
+    )
+    setHeader(event, 'content-length', download.range.end - download.range.start + 1)
+  } else if (download.size !== undefined) {
+    setHeader(event, 'content-length', download.size)
+  }
+
   // Not h3's `sendStream`: its web-stream path neither applies backpressure nor
   // notices the client hanging up, so an aborted download keeps draining the
   // backend read into a socket nobody is reading. `pipeline` destroys the
@@ -32,7 +103,7 @@ export default defineEventHandler(async (event) => {
   // stream carries.
   event._handled = true
   try {
-    await pipeline(stream, event.node.res)
+    await pipeline(download.stream, event.node.res)
   } catch (err) {
     // The client went away mid-body. Expected on long downloads (cancelled
     // jobs, parallel runners) and there is nowhere left to report it.
diff --git a/tests/cleanup-lifecycle.test.ts b/tests/cleanup-lifecycle.test.ts
index c35f48c..dac02ea 100644
--- a/tests/cleanup-lifecycle.test.ts
+++ b/tests/cleanup-lifecycle.test.ts
@@ -249,7 +249,7 @@ describe('cleanup lifecycle', () => {
       const activePartsDownload = await storage.download(entryId)
       expect(mergingDownload).toBeDefined()
       expect(activePartsDownload).toBeDefined()
-      for await (const _chunk of mergingDownload!) void _chunk
+      for await (const _chunk of mergingDownload!.stream) void _chunk
       await storage.waitForOngoingMerges()
 
       const taskModule = await import('~/tasks/cleanup/parts')
@@ -257,7 +257,7 @@ describe('cleanup lifecycle', () => {
       await task.run({} as never)
       expect(await storage.adapter.countFilesInFolder(`${folderName}/parts`)).toBe(1)
 
-      for await (const _chunk of activePartsDownload!) void _chunk
+      for await (const _chunk of activePartsDownload!.stream) void _chunk
 
       await vi.waitFor(
         async () => {
@@ -270,7 +270,7 @@ describe('cleanup lifecycle', () => {
       const mergedDownload = await storage.download(entryId)
       expect(mergedDownload).toBeDefined()
       let restored = ''
-      for await (const chunk of mergedDownload!) restored += chunk.toString()
+      for await (const chunk of mergedDownload!.stream) restored += chunk.toString()
       expect(restored).toBe('cache-data')
     } finally {
       await db.deleteFrom('storage_locations').where('id', '=', locationId).execute()
@@ -320,7 +320,7 @@ describe('cleanup lifecycle', () => {
     await task.run({} as never)
     expect(await storage.adapter.countFilesInFolder(folderName)).toBe(1)
 
-    for await (const _chunk of download!) void _chunk
+    for await (const _chunk of download!.stream) void _chunk
 
     await vi.waitFor(
       async () => {
@@ -446,7 +446,7 @@ describe('cleanup lifecycle', () => {
     try {
       const download = await storage.download(entryId)
       expect(download).toBeDefined()
-      download!.on('error', () => undefined)
+      download!.stream.on('error', () => undefined)
       await db
         .deleteFrom('storage_reader_leases')
         .where('storageLocationId', '=', locationId)
@@ -456,9 +456,10 @@ describe('cleanup lifecycle', () => {
       // The renewal fires on the fake timer, but the lease-lost DB query resolves on a
       // real round-trip — wait for the resulting destroy instead of asserting synchronously.
       // Plain 'close' wait (not events.once, which rejects on the error-destroy).
-      if (!download!.destroyed) await new Promise((resolve) => download!.once('close', resolve))
+      if (!download!.stream.destroyed)
+        await new Promise((resolve) => download!.stream.once('close', resolve))
 
-      expect(download!.destroyed).toBe(true)
+      expect(download!.stream.destroyed).toBe(true)
     } finally {
       vi.useRealTimers()
       await db.deleteFrom('storage_locations').where('id', '=', locationId).execute()
diff --git a/tests/download-merge-backpressure.test.ts b/tests/download-merge-backpressure.test.ts
index d15b410..84c3ab9 100644
--- a/tests/download-merge-backpressure.test.ts
+++ b/tests/download-merge-backpressure.test.ts
@@ -30,7 +30,7 @@ describe('download merge backpressure', () => {
         function* chunked() {
           for (let o = 0; o < buf.length; o += 64 * 1024) yield buf.subarray(o, o + 64 * 1024)
         }
-        return Promise.resolve(Readable.from(chunked()))
+        return Promise.resolve({ stream: Readable.from(chunked()) })
       },
     } as unknown as StorageAdapter
 
diff --git a/tests/eager-merge.test.ts b/tests/eager-merge.test.ts
index 5f37bb0..1f9fa22 100644
--- a/tests/eager-merge.test.ts
+++ b/tests/eager-merge.test.ts
@@ -29,7 +29,7 @@ async function uploadParts(storage: Storage, parts: Buffer[]) {
 }
 
 async function mergedBytes(storage: Storage, folderName: string) {
-  const stream = await storage.adapter.createDownloadStream(`${folderName}/merged`)
+  const { stream } = await storage.adapter.createDownloadStream(`${folderName}/merged`)
   return Buffer.concat(await stream.toArray())
 }
 
diff --git a/tests/range-download.test.ts b/tests/range-download.test.ts
new file mode 100644
index 0000000..60fd5ce
--- /dev/null
+++ b/tests/range-download.test.ts
@@ -0,0 +1,127 @@
+import { Buffer } from 'node:buffer'
+import crypto from 'node:crypto'
+import { Readable } from 'node:stream'
+
+import { describe, expect, test } from 'vitest'
+import { getStorage } from '~/lib/storage'
+
+const SCOPE = 'refs/heads/main'
+const REPO_ID = '123'
+const VERSION = 'range-test-version'
+const DOWNLOAD_URL = 'http://localhost:3000/download'
+
+// Odd size on purpose: not a multiple of any chunk size a client would pick.
+const PART_SIZE = 512 * 1024
+const TOTAL = 2 * PART_SIZE + 12_345
+
+async function createEntry() {
+  const storage = await getStorage()
+  const key = `range-${crypto.randomUUID()}`
+  const part0 = crypto.randomBytes(PART_SIZE)
+  const part1 = crypto.randomBytes(PART_SIZE)
+  const part2 = crypto.randomBytes(TOTAL - 2 * PART_SIZE)
+  const expected = Buffer.concat([part0, part1, part2])
+
+  const upload = await storage.createUpload({
+    key,
+    version: VERSION,
+    scope: SCOPE,
+    repoId: REPO_ID,
+  })
+  if (!upload) throw new Error('createUpload returned nothing')
+  await storage.uploadPart(upload.id, 0, Readable.toWeb(Readable.from(part0)))
+  await storage.uploadPart(upload.id, 1, Readable.toWeb(Readable.from(part1)))
+  await storage.uploadPart(upload.id, 2, Readable.toWeb(Readable.from(part2)))
+  await storage.completeUpload({ key, version: VERSION, scope: SCOPE, repoId: REPO_ID })
+
+  const matched = await storage.matchCacheEntry({
+    keys: [key],
+    version: VERSION,
+    scopes: [SCOPE],
+    repoId: REPO_ID,
+  })
+  const id = matched?.match.id
+  if (!id) throw new Error('cache entry not found after completeUpload')
+  return { id, expected, storage }
+}
+
+async function createMergedEntry() {
+  const entry = await createEntry()
+  // First download of an unmerged entry starts the merge in the background.
+  const first = await entry.storage.download(entry.id)
+  for await (const _ of first!.stream) {
+    /* drain */
+  }
+  await entry.storage.waitForOngoingMerges()
+  return entry
+}
+
+async function get(id: string, range?: string) {
+  const res = await fetch(`${DOWNLOAD_URL}/${id}`, {
+    headers: range ? { range } : {},
+  })
+  const body = Buffer.from(await res.arrayBuffer())
+  return { res, body }
+}
+
+describe('proxy download route serves HTTP Range on merged entries', () => {
+  test('no Range: 200, full body, content-length, accept-ranges', async () => {
+    const { id, expected } = await createMergedEntry()
+    const { res, body } = await get(id)
+    expect(res.status).toBe(200)
+    expect(res.headers.get('accept-ranges')).toBe('bytes')
+    expect(res.headers.get('content-length')).toBe(String(TOTAL))
+    expect(body.compare(expected)).toBe(0)
+  })
+
+  test('closed range: 206 with exact content-range and content-length', async () => {
+    const { id, expected } = await createMergedEntry()
+    const { res, body } = await get(id, 'bytes=4096-8191')
+    expect(res.status).toBe(206)
+    expect(res.headers.get('content-range')).toBe(`bytes 4096-8191/${TOTAL}`)
+    expect(res.headers.get('content-length')).toBe('4096')
+    expect(body.compare(expected.subarray(4096, 8192))).toBe(0)
+  })
+
+  test('open-ended range is clamped to the object', async () => {
+    const { id, expected } = await createMergedEntry()
+    const start = TOTAL - 100
+    const { res, body } = await get(id, `bytes=${start}-`)
+    expect(res.status).toBe(206)
+    expect(res.headers.get('content-range')).toBe(`bytes ${start}-${TOTAL - 1}/${TOTAL}`)
+    expect(body.compare(expected.subarray(start))).toBe(0)
+  })
+
+  test('closed range past the end is clamped, not rejected', async () => {
+    const { id, expected } = await createMergedEntry()
+    const start = TOTAL - 10
+    const { res, body } = await get(id, `bytes=${start}-${TOTAL + 5000}`)
+    expect(res.status).toBe(206)
+    expect(res.headers.get('content-range')).toBe(`bytes ${start}-${TOTAL - 1}/${TOTAL}`)
+    expect(body.compare(expected.subarray(start))).toBe(0)
+  })
+
+  test('range starting at the end: 416 with content-range bytes */size', async () => {
+    const { id } = await createMergedEntry()
+    const { res } = await get(id, `bytes=${TOTAL}-`)
+    expect(res.status).toBe(416)
+    expect(res.headers.get('content-range')).toBe(`bytes */${TOTAL}`)
+  })
+
+  test('unparseable Range falls through to a plain 200', async () => {
+    const { id, expected } = await createMergedEntry()
+    const { res, body } = await get(id, 'bytes=abc')
+    expect(res.status).toBe(200)
+    expect(body.compare(expected)).toBe(0)
+  })
+})
+
+describe('proxy download route on unmerged entries', () => {
+  test('range is ignored: 200 with the full body streamed from parts', async () => {
+    const { id, expected } = await createEntry()
+    const { res, body } = await get(id, 'bytes=0-10')
+    expect(res.status).toBe(200)
+    expect(res.headers.get('content-range')).toBeNull()
+    expect(body.compare(expected)).toBe(0)
+  })
+})
diff --git a/tests/storage-lifecycle.test.ts b/tests/storage-lifecycle.test.ts
index 7201cc0..7e98903 100644
--- a/tests/storage-lifecycle.test.ts
+++ b/tests/storage-lifecycle.test.ts
@@ -37,7 +37,7 @@ describe('storage lifecycle reconciliation', () => {
     let deleteCalls = 0
     const adapter = {
       async createDownloadStream() {
-        return Readable.from('')
+        return { stream: Readable.from('') }
       },
       async uploadStream() {},
       async objectExists() {
@@ -69,7 +69,7 @@ describe('storage lifecycle reconciliation', () => {
     const db = await getDatabase()
     const adapter = {
       async createDownloadStream() {
-        return Readable.from('')
+        return { stream: Readable.from('') }
       },
       async uploadStream() {},
       async objectExists() {

From 9e99d2b92913388d9eb70846c7344830213b3231 Mon Sep 17 00:00:00 2001
From: Peter Svensson <peter@sparetimecoders.com>
Date: Mon, 14 Sep 2026 16:47:07 +0200
Subject: [PATCH 3/4] docs(download): trim comments to what the code cannot say

Most of the comments added in this branch restated the code or the types.
Remove those and keep only the ones explaining a non-obvious why, one line
each: why unmerged entries ignore Range, why the response is taken over from
h3, and why `ActualObjectSize` cannot be relied on everywhere.

The benchmark numbers and the actions/toolkit reference above `parseRange`
move to the PR description, where they will not go stale in the code.

No behaviour change: comments only.
---
 lib/storage.ts                    | 44 +++++------------------------
 routes/download/[cacheEntryId].ts | 46 ++++---------------------------
 2 files changed, 13 insertions(+), 77 deletions(-)

diff --git a/lib/storage.ts b/lib/storage.ts
index 8586d0b..aa11cb2 100644
--- a/lib/storage.ts
+++ b/lib/storage.ts
@@ -65,11 +65,7 @@ export class ObjectNotFoundError extends Error {
   }
 }
 
-/**
- * The requested range starts at or beyond the end of the object (HTTP 416).
- * `size` is the object's size when the backend told us and is omitted
- * otherwise, so the route never advertises a made-up `content-range: bytes *\/0`.
- */
+/** `size` is omitted when the backend didn't report one, so the route can skip `content-range`. */
 export class RangeNotSatisfiableError extends Error {
   constructor(
     objectName: string,
@@ -80,28 +76,18 @@ export class RangeNotSatisfiableError extends Error {
   }
 }
 
-/**
- * A range as requested: inclusive `start-end`, or open-ended (`bytes=N-`) when
- * `end` is omitted. Passed to the backend as-is so the backend, not this
- * server, resolves the end against the object length.
- */
+/** An omitted `end` means open-ended; the backend resolves it against the object length. */
 export interface RangeRequest {
   start: number
   end?: number
 }
 
-/** A resolved, inclusive byte range: what was actually served. */
 export interface ByteRange {
   start: number
   end: number
 }
 
-/**
- * What an adapter hands back for a download. `range` is the range actually
- * served (clamped to the object), present only when the request asked for one
- * AND the adapter honoured it — the route turns that into a 206 with
- * `content-range`. `size` is the whole object's size when known.
- */
+/** `range` is set only when a range was requested and the adapter honoured it. */
 export interface DownloadStream {
   stream: Readable
   size?: number
@@ -184,9 +170,8 @@ export class Storage {
       return this.adapter.createDownloadStream(`${location.folderName}/merged`, range)
 
     await this.ensurePartsExist(location)
-    // No `range` here: an unmerged entry is concatenated from its Parts as it
-    // is read, so there is nothing to seek into. The route serves those whole
-    // under a 200, which is always a correct answer to a Range request.
+    // No `range`: an unmerged entry is concatenated from its parts as it is read, so
+    // there is nothing to seek into. Served whole under a 200, which Range allows.
     return { stream: Readable.from(this.streamParts(location)) }
   }
 
@@ -1051,23 +1036,14 @@ class S3Adapter implements StorageAdapter {
         new GetObjectCommand({
           Bucket: this.bucket,
           Key: `${this.keyPrefix}/${objectName}`,
-          // Passed straight through to S3 rather than sliced here: the point is
-          // to avoid pulling a whole object into the server to serve a slice of
-          // it. An open-ended request stays open-ended (`bytes=N-`) so S3
-          // resolves the end itself and we never depend on it clamping a
-          // sentinel we made up.
           Range: range ? `bytes=${range.start}-${range.end ?? ''}` : undefined,
         }),
       )
       if (!response.Body) throw new Error('No body in S3 get object response')
 
       const stream = response.Body as Readable
-      // 200 means S3 served the whole object: no Range, or one it ignored.
       if (response.$metadata.httpStatusCode !== 206) return { stream, size: response.ContentLength }
 
-      // 206 means the body is a slice, and `bytes <start>-<end>/<size>` is the
-      // only description of which slice. Without it the slice could only go out
-      // under a 200, where a client cannot tell it from the whole object.
       const served = parseContentRange(response.ContentRange)
       if (!served) {
         stream.destroy()
@@ -1395,9 +1371,7 @@ class GcsAdapter implements StorageAdapter {
 
   async createDownloadStream(objectName: string, range?: RangeRequest): Promise<DownloadStream> {
     const file = this.bucket.file(`${this.keyPrefix}/${objectName}`)
-    // `getMetadata` both proves the object exists and carries its size, so the
-    // whole-object path reports `size` like the other adapters do instead of
-    // paying a second round-trip for it.
+    // `getMetadata` proves existence and carries the size, so no second round-trip.
     let size: number
     try {
       const [metadata] = await file.getMetadata()
@@ -1495,7 +1469,6 @@ class GcsAdapter implements StorageAdapter {
   }
 }
 
-/** Resolve a request against an object of `size` bytes; undefined when it starts past the end. */
 function clampRange(range: RangeRequest, size: number): ByteRange | undefined {
   if (range.start >= size) return
   return { start: range.start, end: Math.min(range.end ?? size - 1, size - 1) }
@@ -1510,10 +1483,7 @@ function parseContentRange(header: string | undefined) {
   return { start: Number(m[1]), end: Number(m[2]), size: Number(m[3]) }
 }
 
-/**
- * S3's InvalidRange error carries the object size as `ActualObjectSize`
- * (AWS and MinIO do; not verified on every S3-compatible implementation).
- */
+// AWS and MinIO put the object size here; other S3-compatible servers may not.
 function parseActualObjectSize(err: unknown): number | undefined {
   const raw = (err as { ActualObjectSize?: unknown }).ActualObjectSize
   const size = Number(raw)
diff --git a/routes/download/[cacheEntryId].ts b/routes/download/[cacheEntryId].ts
index fdae960..bac64d1 100644
--- a/routes/download/[cacheEntryId].ts
+++ b/routes/download/[cacheEntryId].ts
@@ -8,16 +8,7 @@ const pathParamsSchema = z.object({
   cacheEntryId: z.string(),
 })
 
-/**
- * `Range: bytes=<start>-<end>`, where `end` may be omitted.
- *
- * Deliberately narrow: this serves one range, not the multi-range or suffix
- * (`bytes=-500`) forms the RFC also allows. Clients of this server ask for
- * closed `start-end` blocks or an open-ended tail and nothing else, and
- * anything unrecognised falls through to a normal 200 with the whole object,
- * which is always correct.
- */
-// The range unit is case-insensitive per RFC 9110 section 14.1.
+// Single ranges only; multi-range and suffix (`bytes=-500`) fall through to a 200.
 const RANGE_RE = /^bytes=(\d+)-(\d*)$/i
 
 function parseRange(header: string | undefined): RangeRequest | undefined {
@@ -26,8 +17,6 @@ function parseRange(header: string | undefined): RangeRequest | undefined {
   if (!m) return
   const start = Number(m[1])
   if (!Number.isSafeInteger(start)) return
-  // Open-ended stays open-ended: the backend resolves the end against the
-  // object, so the length is never needed here and no HEAD is ever issued.
   if (m[2] === '') return { start }
   const end = Number(m[2])
   if (!Number.isSafeInteger(end) || end < start) return
@@ -45,23 +34,9 @@ export default defineEventHandler(async (event) => {
   const { cacheEntryId } = parsedPathParams.data
   const storage = await getStorage()
 
-  // Why this route understands Range at all: `@actions/cache` picks its
-  // download strategy from the URL's HOSTNAME. `.blob.core.windows.net` gets
-  // the concurrent, ranged downloader; everything else gets a single
-  // `httpClient.get()` with no Range and no keep-alive (actions/toolkit,
-  // packages/cache/src/internal/cacheHttpClient.ts). A self-hosted server can
-  // never match that hostname, so its clients are pinned to one stream no
-  // matter how much bandwidth is on the wire. Measured on one runner pod
-  // against one 320 MB object: ~15 MB/s as shipped, 143 MB/s over 8 parallel
-  // ranges. Serving Range here is what lets a range-capable client reach that
-  // without handing object-store credentials to the job — the credentials stay
-  // in this process, which is the point of proxying rather than presigning.
   const range = parseRange(getHeader(event, 'range'))
 
-  // Advertised even on whole-object responses so a client can discover support
-  // from any prior request instead of probing. An UNMERGED entry is streamed
-  // from its Parts and ignores Range (200, full body), so a client must key off
-  // the response status, not this header.
+  // Unmerged entries ignore Range, so clients must key off the status, not this header.
   setHeader(event, 'accept-ranges', 'bytes')
 
   let download
@@ -82,9 +57,6 @@ export default defineEventHandler(async (event) => {
     })
 
   if (range && download.range && download.size !== undefined) {
-    // 206 only when the adapter actually served the (clamped) range. These
-    // headers are how a ranged client learns the total size and verifies each
-    // part, so they must describe exactly what is on the wire.
     setResponseStatus(event, 206)
     setHeader(
       event,
@@ -96,24 +68,18 @@ export default defineEventHandler(async (event) => {
     setHeader(event, 'content-length', download.size)
   }
 
-  // Not h3's `sendStream`: its web-stream path neither applies backpressure nor
-  // notices the client hanging up, so an aborted download keeps draining the
-  // backend read into a socket nobody is reading. `pipeline` destroys the
-  // source when the response closes, which also releases the reader lease the
-  // stream carries.
+  // Take over the response from h3: `sendStream` applies no backpressure and does not
+  // notice client aborts, whereas `pipeline` destroys the source and releases its lease.
   event._handled = true
   try {
     await pipeline(download.stream, event.node.res)
   } catch (err) {
-    // The client went away mid-body. Expected on long downloads (cancelled
-    // jobs, parallel runners) and there is nowhere left to report it.
+    // Client went away mid-body. Expected on cancelled jobs and parallel runners.
     if ((err as NodeJS.ErrnoException).code === 'ERR_STREAM_PREMATURE_CLOSE') {
       logger.debug(`Client aborted /download/${cacheEntryId}: ${(err as Error).message}`)
       return
     }
-    // Headers are already out, so this cannot become an HTTP error response;
-    // Nitro's handler would call `setResponseHeaders` after the fact and crash
-    // with ERR_HTTP_HEADERS_SENT.
+    // Headers are out, so Nitro's error handler would crash with ERR_HTTP_HEADERS_SENT.
     if (event.node.res.headersSent) {
       logger.error(`Download stream failed for ${cacheEntryId}`, { error: err })
       return

From b71e6f54509d689ff56318c1dd83f4410e72abcf Mon Sep 17 00:00:00 2001
From: Peter Svensson <peter@sparetimecoders.com>
Date: Mon, 14 Sep 2026 17:34:00 +0200
Subject: [PATCH 4/4] fix(download): unify empty-object range handling, correct
 the lease test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three review follow-ups.

An empty object has no satisfiable range, but S3 answers `bytes=0-` with an
empty 200 while the filesystem and GCS adapters raise. Normalise on the 416
path in the route so the response does not depend on which backend is
configured.

Note that `event._handled` is an h3 v1 internal, so the h3 v2 upgrade finds it.

Rename `download-abort.test.ts` to `download-leases.test.ts` and correct what
it claims. It asserts the reader lease is released after an abort and after a
completed download, which is real but is not the regression test the previous
name and commit message implied: the server runs as its own process, so the
backend read cannot be observed from the test, and `protectDownloadStream`
releases the lease on `end` as well as `close`, so a finite body clears it
whether or not the abort destroyed anything. Verified by reverting the route
to `sendStream` — the old test passed against the bug it was said to catch.
The docblock now says what is and is not covered.
---
 routes/download/[cacheEntryId].ts |   8 +++
 tests/download-abort.test.ts      |  69 ------------------
 tests/download-leases.test.ts     | 114 ++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 69 deletions(-)
 delete mode 100644 tests/download-abort.test.ts
 create mode 100644 tests/download-leases.test.ts

diff --git a/routes/download/[cacheEntryId].ts b/routes/download/[cacheEntryId].ts
index bac64d1..6d201e3 100644
--- a/routes/download/[cacheEntryId].ts
+++ b/routes/download/[cacheEntryId].ts
@@ -44,6 +44,13 @@ export default defineEventHandler(async (event) => {
     download = await storage.download(cacheEntryId, range)
   } catch (err) {
     if (err instanceof RangeNotSatisfiableError) {
+      // An empty object has no satisfiable range, but S3 answers `bytes=0-` with an
+      // empty 200 while the other adapters raise. Normalise to the 200 so the
+      // response does not depend on which backend is configured.
+      if (err.size === 0) {
+        setHeader(event, 'content-length', 0)
+        return send(event)
+      }
       setResponseStatus(event, 416, 'Range Not Satisfiable')
       if (err.size !== undefined) setHeader(event, 'content-range', `bytes */${err.size}`)
       return send(event)
@@ -70,6 +77,7 @@ export default defineEventHandler(async (event) => {
 
   // Take over the response from h3: `sendStream` applies no backpressure and does not
   // notice client aborts, whereas `pipeline` destroys the source and releases its lease.
+  // `_handled` is an h3 v1 internal and will need replacing when h3 v2 lands.
   event._handled = true
   try {
     await pipeline(download.stream, event.node.res)
diff --git a/tests/download-abort.test.ts b/tests/download-abort.test.ts
deleted file mode 100644
index 600eda9..0000000
--- a/tests/download-abort.test.ts
+++ /dev/null
@@ -1,69 +0,0 @@
-import { Buffer } from 'node:buffer'
-import { createServer } from 'node:http'
-import { Readable } from 'node:stream'
-import { pipeline } from 'node:stream/promises'
-
-import { describe, expect, test, vi } from 'vitest'
-
-/**
- * A source that never ends on its own, and records being destroyed. Stands in
- * for the adapter's backend read (an S3 GET, a file read): the thing that must
- * stop when the client goes away.
- */
-function endlessSource() {
-  const state = { destroyed: false, pushed: 0 }
-  // Paced and unending, so an abort lands mid-body rather than after the whole
-  // object has already been buffered out.
-  async function* chunks() {
-    for (;;) {
-      await new Promise((resolve) => setTimeout(resolve, 5))
-      state.pushed += 1
-      yield Buffer.alloc(64 * 1024)
-    }
-  }
-  const stream = Readable.from(chunks(), { highWaterMark: 16 * 1024 })
-  stream.on('close', () => (state.destroyed = stream.destroyed))
-  return { stream, state }
-}
-
-async function serveOnce(handler: (res: import('node:http').ServerResponse) => void) {
-  const server = createServer((_req, res) => handler(res))
-  await new Promise<void>((resolve) => server.listen(0, resolve))
-  const { port } = server.address() as { port: number }
-  return {
-    url: `http://localhost:${port}/`,
-    close: () => new Promise<void>((resolve) => server.close(() => resolve())),
-  }
-}
-
-/**
- * Regression for the leak this route used to have: h3's `sendStream` hands the
- * body to a web stream that neither applies backpressure nor notices the client
- * hanging up, so an aborted download keeps draining the backend read into a
- * socket nobody is reading — one leaked backend GET per abort, and the reader
- * lease attached to that stream held until it expires instead of being released
- * on close.
- */
-describe('aborted download', () => {
-  test('piping with stream.pipeline destroys the source when the client aborts', async () => {
-    const { stream, state } = endlessSource()
-    const fixture = await serveOnce((res) => {
-      void pipeline(stream, res).catch(() => {
-        // ERR_STREAM_PREMATURE_CLOSE — the client went away, nothing to report.
-      })
-    })
-
-    try {
-      const controller = new AbortController()
-      const res = await fetch(fixture.url, { signal: controller.signal })
-      const reader = res.body!.getReader()
-      await reader.read()
-      controller.abort()
-      await reader.cancel().catch(() => {})
-
-      await vi.waitFor(() => expect(state.destroyed).toBe(true), { timeout: 5000, interval: 25 })
-    } finally {
-      await fixture.close()
-    }
-  })
-})
diff --git a/tests/download-leases.test.ts b/tests/download-leases.test.ts
new file mode 100644
index 0000000..fcb2b77
--- /dev/null
+++ b/tests/download-leases.test.ts
@@ -0,0 +1,114 @@
+import { Buffer } from 'node:buffer'
+import crypto from 'node:crypto'
+import { Readable } from 'node:stream'
+
+import { describe, expect, test, vi } from 'vitest'
+import { getDatabase } from '~/lib/db'
+import { getStorage } from '~/lib/storage'
+
+const SCOPE = 'refs/heads/main'
+const REPO_ID = '123'
+const VERSION = 'abort-test-version'
+const DOWNLOAD_URL = 'http://localhost:3000/download'
+
+// Big enough that the body is still streaming when the client goes away: the
+// abort has to land mid-download for the leak to be observable at all.
+const PART_SIZE = 4 * 1024 * 1024
+const PART_COUNT = 4
+
+async function createEntry() {
+  const storage = await getStorage()
+  const key = `abort-${crypto.randomUUID()}`
+
+  const upload = await storage.createUpload({
+    key,
+    version: VERSION,
+    scope: SCOPE,
+    repoId: REPO_ID,
+  })
+  if (!upload) throw new Error('createUpload returned nothing')
+  for (let i = 0; i < PART_COUNT; i++) {
+    await storage.uploadPart(
+      upload.id,
+      i,
+      Readable.toWeb(Readable.from(crypto.randomBytes(PART_SIZE))),
+    )
+  }
+  await storage.completeUpload({ key, version: VERSION, scope: SCOPE, repoId: REPO_ID })
+
+  const matched = await storage.matchCacheEntry({
+    keys: [key],
+    version: VERSION,
+    scopes: [SCOPE],
+    repoId: REPO_ID,
+  })
+  const id = matched?.match.id
+  if (!id) throw new Error('cache entry not found after completeUpload')
+  return id
+}
+
+async function leaseCount(cacheEntryId: string) {
+  const db = await getDatabase()
+  const rows = await db
+    .selectFrom('storage_reader_leases')
+    .innerJoin(
+      'storage_locations',
+      'storage_locations.id',
+      'storage_reader_leases.storageLocationId',
+    )
+    .innerJoin('cache_entries', 'cache_entries.locationId', 'storage_locations.id')
+    .where('cache_entries.id', '=', cacheEntryId)
+    .select('storage_reader_leases.id')
+    .execute()
+  return rows.length
+}
+
+/**
+ * Lease lifecycle for the download route: the reader lease taken for a download
+ * must be gone once the response is over, whether the client read it to the end
+ * or hung up partway.
+ *
+ * This does NOT prove the abort case destroys the backend read. The server runs
+ * as its own process, so the adapter's read cannot be observed from here, and
+ * `protectDownloadStream` releases the lease on `end` as well as `close` — a
+ * body this size finishes on the server regardless of when the client aborts.
+ * The leak this route used to have (h3's `sendStream` draining the backend into
+ * a socket nobody reads) is not covered by anything here; catching it needs a
+ * body that never ends, which the server would have to be asked to produce.
+ */
+describe('download reader leases', () => {
+  test('releases the reader lease when the client aborts mid-body', async () => {
+    const cacheEntryId = await createEntry()
+
+    const controller = new AbortController()
+    const res = await fetch(`${DOWNLOAD_URL}/${cacheEntryId}`, { signal: controller.signal })
+    expect(res.status).toBe(200)
+
+    const reader = res.body!.getReader()
+    const first = await reader.read()
+    expect(first.done).toBe(false)
+    expect(await leaseCount(cacheEntryId)).toBe(1)
+
+    controller.abort()
+    await reader.cancel().catch(() => {})
+
+    await vi.waitFor(async () => expect(await leaseCount(cacheEntryId)).toBe(0), {
+      timeout: 5000,
+      interval: 50,
+    })
+  }, 20_000)
+
+  test('releases the reader lease after a completed download', async () => {
+    const cacheEntryId = await createEntry()
+
+    const res = await fetch(`${DOWNLOAD_URL}/${cacheEntryId}`)
+    expect(res.status).toBe(200)
+    const body = Buffer.from(await res.arrayBuffer())
+    expect(body).toHaveLength(PART_SIZE * PART_COUNT)
+
+    await vi.waitFor(async () => expect(await leaseCount(cacheEntryId)).toBe(0), {
+      timeout: 5000,
+      interval: 50,
+    })
+  }, 20_000)
+})