Fix CI: untrack generated hook bundle + mock registry for tarball tests#208
Merged
Merged
Conversation
The fleet hook bundle at .claude/hooks/fleet/_dist/bundle.cjs is a release-only generated artifact. It is already covered by .gitignore (the /.claude/hooks/fleet/_dist/ rule), yet it was still tracked in the index, so the fleet Check job failed both ignored-files-are-untracked and generated-outputs-are-untracked (index vs .gitignore disagree). Remove it from the index with git rm --cached. The working-tree file is kept (a bundle refresh lands it on disk) and stays gitignored.
The is-number tarball suites in tarball.test.mts went through pacote to the live registry.npmjs.org. They were gated behind describeNetworkOnly, but a module-eval race lets one escape the network skip and run while the shared test setup's nock.disableNetConnect() (or a sibling file's) is active, so it fails with 'Disallowed net connect' — the flake that turned CI red on 'should lazy load cacache on first use'. Make the extract/pack tests deterministic and offline: - Serve the is-number@7.0.0 packument and tarball from committed fixtures via nock. The packument's dist.integrity/shasum match the tarball bytes exactly so pacote's integrity check passes. - Thread pacote's documented 'agent: false' passthrough (via withMockedNet) so make-fetch-happen uses Node's default agent; nock 15 (backed by @mswjs/interceptors) cannot intercept the keepalive agent pool otherwise. Same mechanism as the existing 'Arborist' passthrough. - Drive the (spec, callback) overload tests from the local tarball fixture path so they extract offline via pacote's file fetcher, no registry. - Move the now-offline suites out of the describeNetworkOnly gate so they always run. The git-spec test shells out to a real 'git clone' (a subprocess nock can't intercept, and which the leak can't affect), so it stays network-gated. - Allowlist test/fixtures/npm/*.tgz in .gitignore so the tarball fixture can be committed (the *.tgz glob would otherwise ignore it).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes
mainCI green and keeps it durably green. The failing run (30109765260) had two red jobs — 🔎 Check and 🧪 Test — and this PR fixes both, then fixes the shared root cause behind the flake so no other network suite can hit it.The fixes
1. 🔎 Check — untrack the generated fleet hook bundle
.claude/hooks/fleet/_dist/bundle.cjswas tracked in git but is a generated, release-only artifact that.gitignorealready ignores. That disagreement (index says "tracked",.gitignoresays "ignore") trippedignored-files-are-untrackedandgenerated-outputs-are-untracked. Removed from the index withgit rm --cached— the working-tree file is untouched and stays gitignored.2. 🧪 Test — mock the npm registry for the tarball tests (don't skip)
The
is-numbersuites intest/unit/packages/tarball.test.mtsdrove pacote to the realregistry.npmjs.org. They now serveis-number@7.0.0from committed fixtures vianock, so the whole suite runs deterministically offline. The one live-network test that genuinely can't be mocked (a realgit clone) stays network-gated.3. 🧪 Test — wire the repo-tier vitest setup so
[network]suites skip by default (root-cause fix)The flake wasn't unique to tarball. The repo-tier test setup that sets
SOCKET_LIB_SKIP_NETWORK_TESTShad been silently dropped by a directory reorg, so everydescribeNetworkOnlysuite was running live against a fail-closed nock. This commit restores it, which durably fixes the same flake intest/unit/dlx/lockfile.test.mtsandtest/unit/packages/fetch.test.mtstoo.Why the bundle was untracked (fleet generated-artifact rule)
The fleet convention is that generated build outputs are never committed — they are produced at release time and listed in
.gitignore..gitignorealready contains/.claude/hooks/fleet/_dist/, and a bundle refresh dropsbundle.cjsonto disk. But the file had also been added to git's index, so git was simultaneously tracking a file it was told to ignore. The two fleet checks exist to catch exactly this.git rm --cachedremoves the index entry while leaving the on-disk file in place, so both checks pass and nothing about the working tree changes.The tarball flake, and how the registry mock makes it deterministic
The tarball extract/pack tests were wrapped in
describeNetworkOnly, which is meant to skip them unless network testing is opted in. Because the skip env was not being set (see fix #3), the suite ran live — while the shared setup'sbeforeAllhad callednock.disableNetConnect(). The in-process pacote request was then blocked and the test failed withNock: Disallowed net connect for "registry.npmjs.org". That is what turned the run red onshould lazy load cacache on first use.The fix removes the network dependency for these tests entirely:
test/fixtures/npm/is-number-7.0.0.tgzis the real tarball, andpackument-is-number.jsonis a minimal packument whosedist.integrity/dist.shasummatch the tarball bytes exactly — otherwise pacote throwsEINTEGRITY.nockserves both the packument (GET /is-number) and the tarball, following the existing registry-mock convention intest/unit/npm/registry.test.mts.agent: false.nock15 is backed by@mswjs/interceptors, which cannot intercept pacote's requests becausemake-fetch-happenroutes them through a keepalive agent pool. Threading pacote's documentedagent: falsepassthrough (via a smallwithMockedNethelper) makes it use Node's default agent so the interceptors match. This changes nothing about what the tests assert — it mirrors the existingArboristpassthrough.(spec, callback)overload tests pass no options object, so they can't threadagent: false; they use the committed tarball's file path as the spec and extract offline through pacote's file fetcher.describeNetworkOnlyso they always run. The git-spec test shells out to a realgit clone, a subprocessnockcan't intercept and the leak can't affect, so it stays network-gated..gitignorenow allowliststest/fixtures/npm/*.tgzso the tarball fixture can be committed (the*.tgzglob would otherwise ignore it — the same tracked-vs-ignored trap as fix Bump @inquirer/search from 3.1.1 to 3.1.3 #1).The skip-race root cause, and the one-file fix
skip-helpersdecides whether to skip a[network]suite by readingprocess.env['SOCKET_LIB_SKIP_NETWORK_TESTS']once, at import time. That env var is set by the repo-tier vitest setup. The fleet-canonical config wires setup like this:A directory reorg renamed
test/scripts/{fleet,repo}/totest/{fleet,repo}/scripts/. The fleet half (test/fleet/scripts/setup.mts) was created, but the repo half (test/repo/scripts/setup.mts) never was — so theexistsSyncfilter silently dropped it, with no error. The env var was therefore never set,skipNetworkwas alwaysfalse, and everydescribeNetworkOnly/itNetworkOnlysuite ran live against the fleet setup's fail-closednock.disableNetConnect(), failing withDisallowed net connect.The fix creates
test/repo/scripts/setup.mts— the exact path the config expects, and the path the fleet setup's own docstring names as the repo-setup home. It defers to the repo's long-standing.config/vitest-setup-tests.mts, so the skip env is set at setupFile-import time, before any test module (and its import-time network gate) is evaluated.SOCKET_LIB_RUN_NETWORK_TESTS=1still opts the live lane in.Note for a follow-up (belongs in the fleet cascade, not this repo PR): the old
test/scripts/{fleet,repo}/setup.mtsfiles are orphaned leftovers from the reorg, and.gitattributes+ the config's setup comment still reference the oldtest/scripts/...paths. They are inert (nothing loads them), so I left them rather than churn fleet-canonical files here.Verification
pnpm run check --allno longer reportsignored-files-are-untrackedorgenerated-outputs-are-untracked.Disallowed net connectfailures (was 8 acrossdlx/lockfile+fetch, plus the tarball flake). The previously-flaky[network]suites now skip deterministically.dlx/lockfile+fetch+tarball+secretsre-run 5× back-to-back:48 passed | 22 skippedevery time.tarball.test.mtsalone: 21/21, also re-run 5×. Deterministic — the flake is gone.pnpm run lintand the typecheck are clean.(The three
git/findtest failures seen in a local full run are worktree-path artifacts — they assert the checkout dir issocket-lib, but a scratch worktree is named differently. They pass in a normal CI checkout and are unrelated to this PR.)