From 6f75895d2e5724c653c393fc4a886c5442109876 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 02:36:19 +0000 Subject: [PATCH 1/3] test: measure streaming working set via forced GC (Node 26 fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The streaming-memory test asserts s3proxy doesn't buffer the full body. It measured process RSS, which is a GC-lag-driven high-water mark that V8 does not return to the OS after collection — so the ceiling drifted between Node versions and failed on Node 26 (~69MB vs the 50MB bound) despite streaming working correctly. Rework the test to measure the actual retained working set instead: - Force a full GC every ~1MB of chunks and sample external Buffer memory (arrayBuffers), which drops the instant freed buffers are collected. This reflects what s3proxy holds (~2MB under backpressure), not transient garbage, and is stable across Node versions. A buffering regression keeps the body referenced, so forced GC can't reclaim it and the delta climbs toward the 100MB payload. Bound tightened from 80MB to 8MB accordingly. - Acquire the GC trigger programmatically via v8.setFlagsFromString rather than relying on --expose-gc. The prior vitest.config.ts `forks.execArgv` entry never took effect (pool options must live under poolOptions, and even there vitest did not forward the flag to worker forks), so global.gc was always undefined and the original test's GC call was a silent no-op. Drop the dead config entry. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01F3RYg3wG5UhVTCx58596iW --- test/streaming-memory.test.ts | 64 +++++++++++++++++++++++++++++------ vitest.config.ts | 3 -- 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/test/streaming-memory.test.ts b/test/streaming-memory.test.ts index 6fae3ae..743f1fc 100644 --- a/test/streaming-memory.test.ts +++ b/test/streaming-memory.test.ts @@ -1,16 +1,35 @@ import { Readable, Writable } from 'node:stream'; +import v8 from 'node:v8'; +import vm from 'node:vm'; import { GetObjectCommand, HeadBucketCommand, S3Client } from '@aws-sdk/client-s3'; import { mockClient } from 'aws-sdk-client-mock'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { S3Proxy } from '../src/index.js'; import { makeReq } from './helpers/http-mocks.js'; +// Obtain a GC trigger without requiring the runner to launch with --expose-gc. +// Vitest does not reliably forward `--expose-gc` to its worker forks, so expose +// gc programmatically via V8 flags instead. Returns null if unavailable (the +// test then skips the strict bound rather than false-failing on uncollected +// garbage). +function acquireGc(): (() => void) | null { + if (typeof global.gc === 'function') return global.gc.bind(global); + try { + v8.setFlagsFromString('--expose-gc'); + const gc = vm.runInNewContext('gc') as (() => void) | undefined; + v8.setFlagsFromString('--no-expose-gc'); + return typeof gc === 'function' ? gc : null; + } catch { + return null; + } +} + // Runs in the plain `test:unit` pass only — `test:coverage` excludes this file -// (see package.json). The assertion measures process RSS, and V8 coverage -// instrumentation inflates RSS and shifts GC timing enough to push the peak -// delta past the bound (~57-62MB observed vs. the 50MB limit), producing false -// failures that have nothing to do with s3proxy's buffering. Un-instrumented, -// the measurement is stable and meaningful, so the strict bound lives here. +// (see package.json). The assertion samples memory usage, and V8 coverage +// instrumentation perturbs allocation and GC timing enough to make the +// measurement unreliable, producing failures that have nothing to do with +// s3proxy's buffering. Un-instrumented, the measurement is stable and +// meaningful, so the bound lives here. describe('streaming memory bound', () => { const s3Mock = mockClient(S3Client); @@ -25,7 +44,7 @@ describe('streaming memory bound', () => { s3Mock.restore(); }); - it('streams a 100MB body with peak RSS delta < 50MB', async () => { + it('streams a 100MB body without buffering the full payload', async () => { const SIZE = 100 * 1024 * 1024; const CHUNK = 64 * 1024; let remaining = SIZE; @@ -51,10 +70,22 @@ describe('streaming memory bound', () => { const proxy = new S3Proxy({ bucket: 'streaming-test' }); await proxy.init(); - if (typeof global.gc === 'function') global.gc(); - const baseRss = process.memoryUsage().rss; + // Force a full GC every GC_WINDOW chunks so the sampled peak reflects the + // *retained* working set rather than uncollected transient garbage. + const gc = acquireGc(); + const GC_WINDOW = 16; // ~1MB of 64KB chunks between forced collections + + gc?.(); + // Measure external Buffer memory (arrayBuffers), not RSS: the payload lives + // in external Buffer storage, not the JS heap, and RSS is a sticky + // high-water mark that V8 does not return to the OS after GC — which is why + // an RSS bound drifts between Node versions. arrayBuffers drops the instant + // freed buffers are collected, so with periodic forced GC it tracks what + // s3proxy actually holds. + const baseAb = process.memoryUsage().arrayBuffers; let peakDelta = 0; let consumed = 0; + let i = 0; const { stream } = await proxy.fetch(makeReq('/big.bin')); @@ -62,7 +93,8 @@ describe('streaming memory bound', () => { const sink = new Writable({ write(chunk: Buffer, _enc, cb) { consumed += chunk.length; - const delta = process.memoryUsage().rss - baseRss; + if (gc && ++i % GC_WINDOW === 0) gc(); + const delta = process.memoryUsage().arrayBuffers - baseAb; if (delta > peakDelta) peakDelta = delta; cb(); }, @@ -74,6 +106,18 @@ describe('streaming memory bound', () => { }); expect(consumed).toBe(SIZE); - expect(peakDelta).toBeLessThan(50 * 1024 * 1024); + // The invariant under test is that s3proxy streams rather than buffering + // the full body. Under backpressure the pipeline retains only a handful of + // in-flight chunks (~1-2MB observed, dominated by the 1MB GC window); a + // buffering implementation would keep the whole body referenced, so forced + // GC could not reclaim it and the delta would climb toward SIZE (100MB). + // The 8MB bound sits far above the streaming working set and far below a + // buffering regression, and — because the GC cadence is forced rather than + // left to the runtime — holds steady across Node versions. If GC could not + // be acquired, transient garbage never gets collected and the measurement + // is meaningless, so the strict bound is skipped. + if (gc) { + expect(peakDelta).toBeLessThan(8 * 1024 * 1024); + } }); }); diff --git a/vitest.config.ts b/vitest.config.ts index 3655a12..9ea7c1a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -18,8 +18,5 @@ export default defineConfig({ }, }, pool: 'forks', - forks: { - execArgv: ['--expose-gc'], - }, }, }); From 22be7af57067e23d53d57aa479cf4032de337ffb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 02:52:49 +0000 Subject: [PATCH 2/3] ci: bump single-pinned jobs from Node 22 to 24 The audit, smoke, validation, performance, and package-verification jobs, plus the release and manual-release workflows, pinned Node 22. Move them to Node 24 (Active LTS) so the fixed build/publish runners track the current LTS. The test matrix still includes 22 to keep the >=22.13.0 engines floor exercised. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01F3RYg3wG5UhVTCx58596iW --- .github/workflows/manual-release.yml | 2 +- .github/workflows/nodejs.yml | 20 ++++++++++---------- .github/workflows/release.yml | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/manual-release.yml b/.github/workflows/manual-release.yml index 2d0d1b2..cf5948d 100644 --- a/.github/workflows/manual-release.yml +++ b/.github/workflows/manual-release.yml @@ -34,7 +34,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' registry-url: 'https://registry.npmjs.org' diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index d1a5ac3..a1e973a 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -11,10 +11,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -78,10 +78,10 @@ jobs: aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -105,10 +105,10 @@ jobs: aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -140,10 +140,10 @@ jobs: aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -217,10 +217,10 @@ jobs: needs: [test] steps: - uses: actions/checkout@v4 - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 681b8a1..2b3c190 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,7 +26,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 registry-url: 'https://registry.npmjs.org' cache: 'npm' From 7e7fbb5bea6b4e04fe9853af117764499e7fb86b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 03:03:35 +0000 Subject: [PATCH 3/3] ci: run package verification across the Node matrix Package verification exercises version-sensitive behavior (ESM resolution, install/import) and needs no AWS credentials, so run it across [22, 24, 26] rather than a single version. The other single-pinned jobs (audit, smoke, validation, performance) stay on Node 24. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01F3RYg3wG5UhVTCx58596iW --- .github/workflows/nodejs.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index a1e973a..6777e16 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -215,12 +215,18 @@ jobs: name: "Package Verification" runs-on: ubuntu-latest needs: [test] + # Run across the full matrix: packaging is version-sensitive (ESM + # resolution and install/import behavior differ between Node versions), + # and this job needs no AWS credentials, so the extra coverage is cheap. + strategy: + matrix: + node-version: [22, 24, 26] steps: - uses: actions/checkout@v4 - - name: Use Node.js 24 + - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4 with: - node-version: 24 + node-version: ${{ matrix.node-version }} cache: 'npm' - name: Install dependencies run: npm ci