test: fix flakes that force CI re-runs

.github/workflows/interop.yml

@@ -63,71 +63,93 @@ jobs:
       run:
         shell: bash
     steps:
+      # Pin a concrete major instead of an lts/ alias. setup-node resolves an
+      # alias by fetching the actions/node-versions manifest from GitHub with no
+      # retry and no fallback, so a GitHub blip fails the job outright. A
+      # concrete version falls back to downloading from nodejs.org.
       - uses: actions/setup-node@v7
         with:
-          node-version: lts/*
+          node-version: '24'
       - uses: actions/download-artifact@v8
         with:
           name: kubo
           path: cmd/ipfs
       - run: chmod +x cmd/ipfs/ipfs
-      - run: sudo apt update
-      - run: sudo apt install -y libxkbcommon0 libxdamage1 libgbm1 libpango-1.0-0 libcairo2 # dependencies for playwright
       # Cache node_modules based on latest @helia/interop version from npm registry.
       # This ensures we always test against the latest release while still benefiting
       # from caching when the version hasn't changed.
       - name: Get latest @helia/interop version
         id: helia-version
-        run: echo "version=$(npm view @helia/interop version)" >> $GITHUB_OUTPUT
-      - name: Cache helia-interop node_modules
-        uses: actions/cache@v6
+        run: |
+          set -euo pipefail
+          for attempt in 1 2 3; do
+            if version=$(npm view @helia/interop version); then break; fi
+            echo "npm view failed (attempt $attempt of 3), retrying" >&2
+            sleep $((attempt * 5))
+          done
+          if [ -z "${version:-}" ]; then
+            echo "could not resolve the latest @helia/interop version from the npm registry" >&2
+            exit 1
+          fi
+          echo "version=$version" >> "$GITHUB_OUTPUT"
+      # Restore and save are separate so a failed install cannot be cached under
+      # a version key and then reused by every later run on that version.
+      - name: Restore helia-interop node_modules
+        uses: actions/cache/restore@v6
         id: helia-cache
         with:
           path: node_modules
           key: ${{ runner.os }}-helia-interop-${{ steps.helia-version.outputs.version }}
-      - name: Install @helia/interop
+      # Install the exact version the cache key names. Plain `npm install
+      # @helia/interop` is a second `latest` lookup, and helia has published
+      # several interop versions within an hour, so the key could name a version
+      # that is not the one on disk.
+      - name: Install @helia/interop@${{ steps.helia-version.outputs.version }}
         if: steps.helia-cache.outputs.cache-hit != 'true'
-        run: npm install @helia/interop
-      # Recurring @helia/interop regression: the published .aegir.js is unusable
-      # from inside node_modules, in one of two ways.
-      #
-      # 1. Missing: the package ships without .aegir.js in its "files", so
-      #    `aegir test` has no config and discovers no spec files. Happened around
-      #    ipfs/helia#1001 (fixed by ipfs/helia#1003), and again in ipfs/helia#1049
-      #    ("feat!: make libp2p optional"), which dropped it from 11.0.0-11.0.2.
-      # 2. Present but broken: the shipped config globs the TypeScript sources
-      #    (./src/*.spec.ts), written for running tests inside the helia monorepo.
-      #    Node refuses to type-strip .ts files under node_modules
-      #    (ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING, no tsconfig is shipped),
-      #    so mocha dies on the first spec. This is what ipfs/helia#1066 restored
-      #    in 11.0.3.
-      #
-      # In both cases we write a minimal config pointing at the prebuilt
-      # ./dist/src, which is all a `-t node` run needs; the rest of helia's own
-      # config is browser-only setup skipped under node. A shipped config that
-      # does not glob .ts files is left untouched, so this step self-heals once
-      # helia publishes a node_modules-safe config. It writes into node_modules
-      # and is fragile if helia's dist layout shifts.
-      - name: Work around unusable @helia/interop/.aegir.js (ipfs/helia#1049, ipfs/helia#1066)
+        run: npm install --fetch-retries=5 --fetch-retry-maxtimeout=120000 "@helia/interop@${{ steps.helia-version.outputs.version }}"
+      # Search for the compiled specs rather than hardcoding where they live.
+      # Upstream has already moved this stuff around more than once, and every
+      # time it did, this job failed with mocha's unhelpful "no test files
+      # found". Searching survives a move; if the specs stop being called
+      # *.spec.js the job still fails, but it says so in one line.
+      - name: Find the compiled interop specs
+        working-directory: node_modules/@helia/interop
         run: |
-          config=node_modules/@helia/interop/.aegir.js
-          if [ ! -f "$config" ]; then
-            echo "export default { test: { files: './dist/src/*.spec.js' } }" > "$config"
-            echo "injected minimal $config: file missing from published package"
-          elif grep -qE "files:.*\.spec\.ts" "$config"; then
-            echo "export default { test: { files: './dist/src/*.spec.js' } }" > "$config"
-            echo "replaced $config: shipped config globs .ts sources, which node cannot run from node_modules"
-          else
-            echo "$config is usable as shipped, no workaround needed"
+          set -euo pipefail
+          find . -name '*.spec.js' -not -path './node_modules/*' | sort > "$RUNNER_TEMP/helia-specs.txt"
+          count=$(wc -l < "$RUNNER_TEMP/helia-specs.txt")
+          echo "@helia/interop@${{ steps.helia-version.outputs.version }} ships $count compiled spec files:"
+          cat "$RUNNER_TEMP/helia-specs.txt"
+          if [ "$count" -eq 0 ]; then
+            echo "found no compiled *.spec.js in the published package, so its layout changed" >&2
+            exit 1
           fi
+      - name: Save helia-interop node_modules
+        if: steps.helia-cache.outputs.cache-hit != 'true'
+        uses: actions/cache/save@v6
+        with:
+          path: node_modules
+          key: ${{ runner.os }}-helia-interop-${{ steps.helia-version.outputs.version }}
       # We run aegir directly (instead of the helia-interop binary) because the
       # bin spawns a bare `aegir test` with no way to pass the flags below.
+      #
+      # --files: pass the specs we found above rather than trusting the .aegir.js
+      # that @helia/interop publishes, which is written for running inside the
+      # helia monorepo and has broken this job twice. It globs the TypeScript
+      # sources (./src/*.spec.ts), which node refuses to type-strip under
+      # node_modules (ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING, no tsconfig is
+      # shipped), and versions 11.0.0-11.0.2 shipped no config at all
+      # (ipfs/helia#1049), leaving aegir with nothing to discover.
+      #
       # --exit: the node interop specs spawn kubo daemons (via ipfsd-ctl +
       # KUBO_BINARY) whose libp2p/RPC handles keep Node's event loop alive after
       # the run, so mocha prints its summary and then hangs until the job timeout.
       # Force exit once the run completes.
       - name: Run helia-interop tests
-        run: npx aegir test -t node --bail -- --exit
+        run: |
+          set -euo pipefail
+          mapfile -t specs < "$RUNNER_TEMP/helia-specs.txt"
+          npx aegir test -t node --bail --files "${specs[@]}" -- --exit
         env:
           KUBO_BINARY: ${{ github.workspace }}/cmd/ipfs/ipfs
         working-directory: node_modules/@helia/interop

core/corehttp/routing.go

@@ -131,7 +131,11 @@ func (r *contentRouter) GetClosestPeers(ctx context.Context, key cid.Cid) (iter.
 		return nil, fmt.Errorf("GetClosestPeers not supported for DHT type %T", r.n.DHTClient)
 	}
 
-	if err != nil {
+	// A lookup cut short by the deadline returns the closest peers found so far
+	// along with the context error. The HTTP routing server caps every request
+	// (server.DefaultRoutingTimeout), so on a slow query this is the difference
+	// between a useful answer and a 500.
+	if err != nil && len(peers) == 0 {
 		return nil, err
 	}
 

core/coreunix/add_test.go

@@ -7,6 +7,7 @@ import (
 	"math/rand"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 	"time"
 
@@ -29,6 +30,31 @@ import (
 
 const testPeerID = "QmTFauExutTsy4XP6JbMFcw2Wa9645HJt2bTqL6qYDCKfe"
 
+// signalFirstRead closes signal the first time the wrapped reader is read from.
+// The gc tests use it to tell when the adder has moved on to a given file.
+type signalFirstRead struct {
+	r      io.Reader
+	once   sync.Once
+	signal chan struct{}
+}
+
+func (s *signalFirstRead) Read(p []byte) (int, error) {
+	s.once.Do(func() { close(s.signal) })
+	return s.r.Read(p)
+}
+
+// waitForGCRequest blocks until a gc is waiting for the pin lock.
+func waitForGCRequest(ctx context.Context, t *testing.T, locker blockstore.GCLocker) {
+	t.Helper()
+	for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline); {
+		if locker.GCRequested(ctx) {
+			return
+		}
+		time.Sleep(time.Millisecond)
+	}
+	t.Fatal("timed out waiting for gc to request the pin lock")
+}
+
 func TestAddMultipleGCLive(t *testing.T) {
 	ctx := t.Context()
 	r := &repo.Mock{
@@ -84,8 +110,10 @@ func TestAddMultipleGCLive(t *testing.T) {
 		gc1out = gc.GC(ctx, node.Blockstore, node.Repo.Datastore(), node.Pinning, nil)
 	}()
 
-	// Give GC goroutine time to reach GCLock (will block there waiting for adder)
-	time.Sleep(time.Millisecond * 100)
+	// Wait for the GC goroutine to reach GCLock, where it blocks behind the pin
+	// lock the adder holds. Until it gets there the adder has no reason to pause,
+	// and it would run to the end of the next file before yielding.
+	waitForGCRequest(ctx, t, node.Blockstore)
 
 	// GC shouldn't get the lock until after the file is completely added
 	select {
@@ -126,8 +154,8 @@ func TestAddMultipleGCLive(t *testing.T) {
 		gc2out = gc.GC(ctx, node.Blockstore, node.Repo.Datastore(), node.Pinning, nil)
 	}()
 
-	// Give GC goroutine time to reach GCLock
-	time.Sleep(time.Millisecond * 100)
+	// Wait for the GC goroutine to reach GCLock, as above.
+	waitForGCRequest(ctx, t, node.Blockstore)
 
 	select {
 	case <-gc2started:
@@ -187,7 +215,8 @@ func TestAddGCLive(t *testing.T) {
 
 	// make two files with pipes so we can 'pause' the add for timing of the test
 	piper, pipew := io.Pipe()
-	hangfile := files.NewReaderFile(piper)
+	addingHangfile := make(chan struct{})
+	hangfile := files.NewReaderFile(&signalFirstRead{r: piper, signal: addingHangfile})
 
 	rfd := files.NewBytesFile([]byte("testfileD"))
 
@@ -215,13 +244,22 @@ func TestAddGCLive(t *testing.T) {
 		t.Fatal("add shouldn't complete yet")
 	}
 
+	// Wait until the add is inside the hanging file. Between two files the adder
+	// hands the pin lock over to a waiting gc, so asking for gc before this point
+	// lets gc start immediately and the assertions below become meaningless.
+	<-addingHangfile
+
 	var gcout <-chan gc.Result
 	gcstarted := make(chan struct{})
 	go func() {
 		defer close(gcstarted)
 		gcout = gc.GC(ctx, node.Blockstore, node.Repo.Datastore(), node.Pinning, nil)
 	}()
 
+	// Wait for gc to actually queue up behind the pin lock the add holds, so the
+	// add has something to yield to once it finishes the current file.
+	waitForGCRequest(ctx, t, node.Blockstore)
+
 	// gc shouldn't start until we let the add finish its current file.
 	if _, err := pipew.Write([]byte("some data for file b")); err != nil {
 		t.Fatal(err)
@@ -233,8 +271,6 @@ func TestAddGCLive(t *testing.T) {
 	default:
 	}
 
-	time.Sleep(time.Millisecond * 100) // make sure gc gets to requesting lock
-
 	// finish write and unblock gc
 	pipew.Close()
 

docs/examples/kubo-as-a-library/main.go

@@ -2,10 +2,10 @@ package main
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
-	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -81,6 +81,12 @@ func createTempRepo() (string, error) {
 	// No automatic bootstrap: we connect only the peers we need.
 	cfg.Bootstrap = []string{}
 
+	// No local peer discovery either. This example dials its second node by
+	// address, and mDNS would race that: a connection opened while the node is
+	// still being built is invisible to its own bitswap, and the fetch at the
+	// end then waits forever. It would also reach unrelated nodes on your LAN.
+	cfg.Discovery.MDNS.Enabled = false
+
 	// Optional: enable experimental features by modifying cfg before Init, e.g.:
 	if *flagExp {
 		// https://github.com/ipfs/kubo/blob/master/docs/experimental-features.md#ipfs-filestore
@@ -170,18 +176,24 @@ func connectToPeers(ctx context.Context, ipfs icore.CoreAPI, peers []string) err
 		pi.Addrs = append(pi.Addrs, pii.Addrs...)
 	}
 
+	var mu sync.Mutex
+	var errs []error
 	wg.Add(len(peerInfos))
 	for _, peerInfo := range peerInfos {
 		go func(peerInfo *peer.AddrInfo) {
 			defer wg.Done()
 			err := ipfs.Swarm().Connect(ctx, *peerInfo)
 			if err != nil {
-				log.Printf("failed to connect to %s: %s", peerInfo.ID, err)
+				mu.Lock()
+				errs = append(errs, fmt.Errorf("connecting to %s: %w", peerInfo.ID, err))
+				mu.Unlock()
 			}
 		}(peerInfo)
 	}
 	wg.Wait()
-	return nil
+	// Report failures instead of logging them. Everything after this assumes the
+	// peers are reachable, so carrying on would stall somewhere less obvious.
+	return errors.Join(errs...)
 }
 
 func getUnixfsNode(path string) (files.Node, error) {
@@ -209,7 +221,10 @@ func main() {
 
 	fmt.Println("-- Getting an IPFS node running -- ")
 
-	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	// Well inside the 2 minute budget `go test` gives this example, so a stall
+	// ends in a panic naming the step that hung rather than the test harness
+	// killing us with no clue about where we were.
+	ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second)
 	defer cancel()
 
 	// Spawn a local peer using a temporary path, for testing purposes.

docs/examples/kubo-as-a-library/main_test.go

@@ -14,7 +14,9 @@ func TestExample(t *testing.T) {
 	t.Log("Starting go run main.go...")
 	start := time.Now()
 
-	cmd := exec.Command("go", "run", "main.go")
+	// CommandContext so the child is killed with the test instead of being left
+	// behind as an orphan when the example hangs.
+	cmd := exec.CommandContext(t.Context(), "go", "run", "main.go")
 	cmd.Env = append(os.Environ(), "GOLOG_LOG_LEVEL=error") // reduce libp2p noise
 
 	// Stream output to both test log and capture buffer for verification