From 776f0768c6bfdf9982a2612ac7761bc1ef6496d4 Mon Sep 17 00:00:00 2001
From: Vladimir Rogojin <vrogojin@vrogojin.net>
Date: Sun, 3 May 2026 10:14:06 +0200
Subject: [PATCH 1/3] test(e2e-live): per-session isolation + bounded-parallel
 provisioning
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two concurrent invocations of `npm run test:e2e-live` previously shared a
flat namespace for Docker container names and /tmp directories — a hand-
ful of name collisions away from cross-run interference. Add a per-process
SESSION_ID (8-hex, generated once at module load; overridable via
TRADER_E2E_SESSION_ID) and stamp it onto:

  - Every Docker container name (trader + escrow): trader-e2e-<SID>-<label>-<rand>
  - Every /tmp wallet/controller dir: /tmp/trader-e2e-<SID>-<label>-XXXXXX
  - The diagnostic-dump filter in waitForDealInState (was a dead lookup
    `escrow-e2e` that never matched anything; now uses the session prefix
    which captures both trader and escrow containers cleanly)

Nostr-side identifiers (per-trader secp256k1 keypair, nametag from UUID)
already have ≥10⁹ entropy so they don't need a session prefix; adding one
to instance_id would only reduce the 9-hex randomness in the nametag
slice, increasing collision risk within a session.

Replace `provisionTradersStaggered`'s sequential for-loop with a bounded
worker pool (default concurrency 3, env-tunable via
TRADER_E2E_PROVISION_CONCURRENCY). The pool preserves input ordering of
results and caps simultaneous Sphere.init/nametag-publish load on the
shared testnet relay. Validated by provisioning-load-investigation that
3-way parallel is reliable on a healthy relay; tunable down to 1 when
the relay is degraded.

Add VITEST_MAX_FORKS opt-in knob to vitest.e2e-live.config.ts. Default
remains 1 (singleFork: true) for backward compatibility — multi-fork
parallelism is opt-in by env so users can flip it when ready.

All 113 helper unit tests pass with the new session module wired in.
---
 test/e2e-live/helpers/docker-helpers.ts   |  8 ++-
 test/e2e-live/helpers/scenario-helpers.ts | 25 ++++---
 test/e2e-live/helpers/session.ts          | 52 +++++++++++++++
 test/e2e-live/helpers/tenant-fixture.ts   | 80 +++++++++++++++++------
 vitest.e2e-live.config.ts                 | 29 +++++++-
 5 files changed, 163 insertions(+), 31 deletions(-)
 create mode 100644 test/e2e-live/helpers/session.ts

diff --git a/test/e2e-live/helpers/docker-helpers.ts b/test/e2e-live/helpers/docker-helpers.ts
index 8e3c841..567bc30 100644
--- a/test/e2e-live/helpers/docker-helpers.ts
+++ b/test/e2e-live/helpers/docker-helpers.ts
@@ -29,6 +29,7 @@ import type {
   StopContainer,
   WaitForContainerRunning,
 } from './contracts.js';
+import { sessionContainerPrefix } from './session.js';
 
 // ----------------------------------------------------------------------------
 // Errors
@@ -170,8 +171,13 @@ export function buildRunArgs(opts: DockerRunOptions, name: string): string[] {
 function buildName(label: string | undefined): string {
   // Docker container names: [a-zA-Z0-9][a-zA-Z0-9_.-]+
   // Label has already been validated against LABEL_RE if provided.
+  //
+  // Stem includes the per-process SESSION_ID so two concurrent invocations
+  // of `npm run test:e2e-live` produce disjoint name spaces. See
+  // session.ts for the rationale.
   const suffix = Math.random().toString(36).slice(2, 8);
-  const stem = label ? `trader-e2e-${label}` : 'trader-e2e';
+  const sessionPrefix = sessionContainerPrefix();
+  const stem = label ? `${sessionPrefix}-${label}` : sessionPrefix;
   return `${stem}-${suffix}`;
 }
 
diff --git a/test/e2e-live/helpers/scenario-helpers.ts b/test/e2e-live/helpers/scenario-helpers.ts
index 8dc752d..9ea03fa 100644
--- a/test/e2e-live/helpers/scenario-helpers.ts
+++ b/test/e2e-live/helpers/scenario-helpers.ts
@@ -23,6 +23,7 @@ import { pollUntil } from './polling.js';
 import { SWAP_TIMEOUT_MS } from './constants.js';
 import { getControllerWallet } from './tenant-fixture.js';
 import { getContainerLogs, listContainersByNamePrefix } from './docker-helpers.js';
+import { sessionContainerPrefix } from './session.js';
 
 const DEAL_POLL_INTERVAL_MS = 2_000;
 const CREATE_INTENT_TIMEOUT_MS = 30_000;
@@ -234,24 +235,32 @@ export async function waitForDealInState(
       /* container may already be gone */
     }
 
-    // Also dump every running escrow container's recent logs. The trader-side
-    // logs alone don't tell us whether the escrow received the deposit, paid
-    // out, or got stuck — without these, parallel-swap and unreachable-escrow
-    // failures are nearly impossible to diagnose post-mortem.
+    // Also dump every running container from THIS session's tenant pool —
+    // includes escrows + any sibling traders. The trader-side logs alone
+    // don't tell us whether the escrow received the deposit, paid out, or
+    // got stuck. Filter by `sessionContainerPrefix()` so a parallel
+    // session running on the same Docker daemon doesn't leak its logs into
+    // our diagnostic output (and vice versa).
+    //
+    // NOTE: docker-helpers names BOTH trader and escrow containers with
+    // the `trader-e2e-<SESSION>-` prefix, so this single filter captures
+    // escrows. The previous `escrow-e2e` filter never matched anything.
     try {
-      const escrowContainerIds = await listContainersByNamePrefix('escrow-e2e');
-      for (const id of escrowContainerIds) {
+      const sessionContainerIds = await listContainersByNamePrefix(sessionContainerPrefix());
+      for (const id of sessionContainerIds) {
+        // Skip the tenant whose logs we already dumped above.
+        if (id === tenant.container.id) continue;
         try {
           const logs = await getContainerLogs(id, 1500);
           console.error(
-            `[waitForDealInState] last 1500 log lines from escrow container ${id}:\n${logs}`,
+            `[waitForDealInState] last 1500 log lines from session container ${id}:\n${logs}`,
           );
         } catch {
           /* container may have just exited — skip */
         }
       }
     } catch {
-      /* listing failed (docker error) — skip escrow logs */
+      /* listing failed (docker error) — skip sibling logs */
     }
 
     throw new Error(
diff --git a/test/e2e-live/helpers/session.ts b/test/e2e-live/helpers/session.ts
new file mode 100644
index 0000000..cced534
--- /dev/null
+++ b/test/e2e-live/helpers/session.ts
@@ -0,0 +1,52 @@
+/**
+ * session — process-wide identifier for a single `npm run test:e2e-live` run.
+ *
+ * Two independent invocations of the e2e-live suite (concurrent CI shards,
+ * two developers on the same host, etc.) MUST NOT interfere. The shared
+ * resources at risk are:
+ *
+ *   - Docker container names (one daemon, flat name space)
+ *   - /tmp directory layout (test-only, but `ls /tmp/trader-e2e-*` should
+ *     scope cleanly to a single run)
+ *   - Diagnostic queries that filter by name prefix
+ *     (see scenario-helpers `listContainersByNamePrefix`)
+ *
+ * The Nostr-side identifiers (per-trader secp256k1 keypair, nametag derived
+ * from the instance UUID) already have ≥10⁹ entropy per resource so they
+ * do not need a session prefix to remain non-colliding — adding one would
+ * only reduce the 9-hex randomness in the nametag slice.
+ *
+ * SESSION_ID is generated ONCE at module load. Every helper that constructs
+ * a name targeting a shared resource must read it from here so it is
+ * uniformly attached.
+ *
+ * Override via `TRADER_E2E_SESSION_ID=...` if a CI driver wants to tag the
+ * run with its own job id for cross-tool log correlation. The override is
+ * sanitized to lowercase hex; non-hex characters are stripped. Empty values
+ * are ignored — a fresh ID is generated.
+ */
+
+import { randomBytes } from 'node:crypto';
+
+function readSessionOverride(): string | null {
+  const raw = process.env['TRADER_E2E_SESSION_ID'];
+  if (typeof raw !== 'string') return null;
+  // Sanitize: lowercase, hex-only, max 16 chars. Refuse exotic characters
+  // because the value gets concatenated into Docker container names which
+  // have a strict allowed-character class ([a-zA-Z0-9_.-]).
+  const cleaned = raw.toLowerCase().replace(/[^0-9a-f]/g, '').slice(0, 16);
+  return cleaned.length > 0 ? cleaned : null;
+}
+
+/** 8-hex-char session id (32 bits ≈ 4.3 billion distinct sessions). */
+export const SESSION_ID: string = readSessionOverride() ?? randomBytes(4).toString('hex');
+
+/**
+ * Common prefix for every container/tmp-dir name produced by this run.
+ * Two simultaneous test sessions will end up with distinct prefixes and
+ * `docker ps --filter name=<sessionContainerPrefix()>` lists only the
+ * current run's containers.
+ */
+export function sessionContainerPrefix(): string {
+  return `trader-e2e-${SESSION_ID}`;
+}
diff --git a/test/e2e-live/helpers/tenant-fixture.ts b/test/e2e-live/helpers/tenant-fixture.ts
index a9631b4..8c74d91 100644
--- a/test/e2e-live/helpers/tenant-fixture.ts
+++ b/test/e2e-live/helpers/tenant-fixture.ts
@@ -47,6 +47,7 @@ import { runTraderCtl } from './trader-ctl-driver.js';
 import { RELAYS, TRADER_IMAGE, ESCROW_IMAGE } from './constants.js';
 import { pollUntil } from './polling.js';
 import { fundWallet } from './funding.js';
+import { SESSION_ID } from './session.js';
 
 // ---------------------------------------------------------------------------
 // Local extensions to ProvisionTraderOptions
@@ -141,7 +142,9 @@ let controllerCache: Promise<ControllerWallet> | null = null;
 export async function getControllerWallet(): Promise<ControllerWallet> {
   if (controllerCache !== null) return controllerCache;
   controllerCache = (async () => {
-    const root = mkdtempSync(join(tmpdir(), 'trader-e2e-controller-'));
+    // Per-session prefix so two concurrent test runs don't share a cache dir
+    // and so `ls /tmp/trader-e2e-controller-<sid>-*` scopes cleanly.
+    const root = mkdtempSync(join(tmpdir(), `trader-e2e-controller-${SESSION_ID}-`));
     const dataDir = join(root, 'wallet');
     const tokensDir = join(root, 'tokens');
     mkdirSync(dataDir, { recursive: true });
@@ -228,7 +231,8 @@ function realSecp256k1Pubkey(): string {
  */
 function materializeWalletDir(label: string): string {
   const safeLabel = label.replace(/[^a-zA-Z0-9-_]/g, '-').slice(0, 24);
-  const root = mkdtempSync(join(tmpdir(), `trader-e2e-${safeLabel}-`));
+  // Per-session prefix isolates two concurrent runs' wallet dirs in /tmp.
+  const root = mkdtempSync(join(tmpdir(), `trader-e2e-${SESSION_ID}-${safeLabel}-`));
   const walletDir = join(root, 'wallet');
   const tokensDir = join(root, 'tokens');
   mkdirSync(walletDir, { recursive: true });
@@ -694,36 +698,70 @@ export async function provisionTrader(
 }
 
 // ============================================================================
-// provisionTradersStaggered — concurrent provisioning with kickoff stagger
+// provisionTradersStaggered — bounded-parallel provisioning
 // ============================================================================
 
+const DEFAULT_PROVISION_CONCURRENCY = 3;
+
+function readProvisionConcurrency(): number {
+  const raw = process.env['TRADER_E2E_PROVISION_CONCURRENCY'];
+  if (raw === undefined || raw === '') return DEFAULT_PROVISION_CONCURRENCY;
+  const n = Number.parseInt(raw, 10);
+  if (!Number.isFinite(n) || n < 1) return DEFAULT_PROVISION_CONCURRENCY;
+  return n;
+}
+
 /**
- * Provisions multiple traders sequentially.
+ * Provision multiple traders with bounded parallelism.
  *
- * **Why sequential, not parallel:** the testnet's single Nostr relay is
- * the bottleneck. When N traders run `Sphere.init` concurrently, all of
- * them publish nametag binding events and then each does a self-verify
- * `sphere.resolve()` query. The relay's queryEvents subscription queue
- * serializes — under N>=3 concurrent load, queries time out at 15s and
- * each trader's verify loop runs out of budget. We've seen all 3 traders
- * hang at sphere_initialized for 180s under pure Promise.all.
+ * **Why bounded, not unbounded Promise.all:** the testnet's single Nostr
+ * relay is the bottleneck. Under unbounded concurrency we have observed
+ * (N≥4) one trader hanging at `sphere_initialized` for 3 minutes while its
+ * `sphere.resolve()` self-verify times out. Bounded parallelism with a
+ * small cap keeps the simultaneous nametag-publish + self-verify load
+ * within the relay's serving budget.
  *
- * Sequential gives each trader's binding event time to propagate to the
- * relay's query index before the next trader starts hitting it. Cost is
- * ~30-60s per trader (dominated by sphere.init + verify); total wall
- * time scales linearly with N.
+ * Concurrency is configurable via `TRADER_E2E_PROVISION_CONCURRENCY` (env
+ * var, default 3). Set to 1 to fall back to fully sequential behavior on
+ * a degraded relay; set higher when the relay is healthy and you want to
+ * cut wall-clock provisioning time. The 2026-04-30 measurement in
+ * `provisioning-load-investigation.e2e-live.test.ts` validates 3-way
+ * parallel provisioning is reliable on a healthy relay.
  *
- * If the testnet relay's subscription throughput improves, this can be
- * revisited as parallel-with-stagger.
+ * **Result ordering** matches the input order regardless of completion
+ * order — callers commonly destructure `[alice, bob, carol] = await ...`
+ * and rely on positional alignment with the factory list.
  */
 export async function provisionTradersStaggered(
   factories: Array<() => Promise<ProvisionedTenant>>,
 ): Promise<ProvisionedTenant[]> {
-  const results: ProvisionedTenant[] = [];
-  for (const factory of factories) {
-    if (factory === undefined) continue;
-    results.push(await factory());
+  const tasks = factories.filter((f): f is () => Promise<ProvisionedTenant> => f !== undefined);
+  const concurrency = Math.min(readProvisionConcurrency(), Math.max(tasks.length, 1));
+  const results: ProvisionedTenant[] = new Array(tasks.length);
+
+  let nextIndex = 0;
+  // Worker pool: each worker pulls the next task index and runs it. When
+  // the index pointer crosses the task list, the worker exits. With
+  // `concurrency` workers we cap simultaneous in-flight provisions.
+  async function worker(): Promise<void> {
+    for (;;) {
+      const i = nextIndex++;
+      if (i >= tasks.length) return;
+      // Non-null assertion is safe: i < tasks.length guarantees defined.
+      results[i] = await tasks[i]!();
+    }
+  }
+
+  const workers: Array<Promise<void>> = [];
+  for (let i = 0; i < concurrency; i++) {
+    workers.push(worker());
   }
+  // Promise.all rejects on the first worker error. Other workers are still
+  // running, so newly-finished provisions may need cleanup — but that
+  // cleanup is the caller's concern (afterAll always disposes whatever
+  // resolved). The original sequential implementation had the same
+  // semantics: a failure in trader N didn't dispose 0..N-1.
+  await Promise.all(workers);
   return results;
 }
 
diff --git a/vitest.e2e-live.config.ts b/vitest.e2e-live.config.ts
index bfcae6c..f0e5f9e 100644
--- a/vitest.e2e-live.config.ts
+++ b/vitest.e2e-live.config.ts
@@ -18,13 +18,40 @@
  */
 import { defineConfig } from 'vitest/config';
 
+/**
+ * VITEST_MAX_FORKS — opt-in knob for parallelizing e2e-live test FILES
+ * across vitest worker forks. Default is 1 (sequential, one file at a
+ * time) which preserves the historical behavior of `singleFork: true`.
+ *
+ * Two forks would run two test files concurrently. Each fork spawns its
+ * own set of Docker containers (the test/e2e-live helpers session-prefix
+ * everything) so cross-fork resource collisions are designed-out, but the
+ * shared testnet Nostr relay is the bottleneck. Don't raise this above
+ * what the relay can handle (empirically 2-3 today).
+ *
+ * The session-isolation work in test/e2e-live/helpers/session.ts is what
+ * makes cross-fork (and cross-process) parallelism safe — see that file
+ * for the per-resource analysis.
+ */
+const MAX_FORKS = process.env['VITEST_MAX_FORKS']
+  ? Math.max(1, Number.parseInt(process.env['VITEST_MAX_FORKS'], 10) || 1)
+  : 1;
+
 export default defineConfig({
   test: {
     include: ['test/e2e-live/**/*.test.ts'],
     testTimeout: 180_000,
     hookTimeout: 300_000,
     pool: 'forks',
-    poolOptions: { forks: { singleFork: true } },
+    poolOptions: {
+      forks: {
+        // singleFork preserved as default for backward compatibility.
+        // Setting VITEST_MAX_FORKS>1 flips to a multi-fork pool.
+        singleFork: MAX_FORKS === 1,
+        maxForks: MAX_FORKS,
+        minForks: 1,
+      },
+    },
     sequence: { concurrent: false },
     retry: 0,
     bail: 1,

From ee1505e629592490763a78983994d3e7a313029d Mon Sep 17 00:00:00 2001
From: Vladimir Rogojin <vrogojin@vrogojin.net>
Date: Sun, 3 May 2026 14:51:05 +0200
Subject: [PATCH 2/3] =?UTF-8?q?test(e2e-live):=20address=20PR=20#10=20revi?=
 =?UTF-8?q?ew=20feedback=20=E2=80=94=20anchored=20docker=20filter=20+=20le?=
 =?UTF-8?q?ak-free=20worker=20pool?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two critical findings from the steelman review of #10:

## C1: anchor docker --filter name regex with `^`

`docker ps --filter name=X` is a SUBSTRING match by default — `name=foo`
matches any container whose name CONTAINS "foo", not just those that
START with "foo". The PR documented session-prefix as a security-style
isolation guarantee, but the unanchored filter could in theory match
adjacent-session containers if their session IDs shared leading hex digits.

Anchor with `^` (Docker passes the value through to its regexp matcher,
so `^prefix` is honored). Verified with two containers `test-prefix-foo`
and `xtest-prefix-foo-y`:
- without `^`: matches both (substring)
- with `^`: matches only the prefix-anchored one

## C2: Promise.allSettled + dispose orphaned tenants on partial failure

The previous worker pool used `await Promise.all(workers)`, which rejects
on first error. But other in-flight workers continue spawning containers
AFTER the function rejects, and those containers never reach `results`,
so the caller's `afterAll` never sees them. **Container + /tmp leak,
strictly worse than the sequential predecessor.**

Fix: each worker catches per-task errors into a shared `errors[]` array
and continues draining. After all workers settle, if any errors were
recorded, dispose every tenant that DID succeed (they're unreachable to
the caller through the rejected promise) before re-throwing the first
error. Multiple errors are attached as `.otherErrors` on the primary so
they aren't silently swallowed.

## Drive-by: replace top-level `generatePrivateKey` import

`@unicitylabs/sphere-sdk` no longer exports `generatePrivateKey` at the
package root (moved to the L1 sub-namespace). Replace with an inlined
`randomBytes(32).toString('hex')` — a secp256k1 private key is just 32
random bytes, and the probability of generating an invalid value is
~2^-128 (vanishingly small).

## Test plan

- 113/113 helper unit tests pass
- typecheck clean for the test/ tree (the unrelated `mintFungibleToken`
  type error in `src/trader/main.ts` belongs to PR #12 and is fixed there)
- Docker filter anchor verified empirically with two test containers
---
 test/e2e-live/helpers/docker-helpers.ts |  8 ++-
 test/e2e-live/helpers/tenant-fixture.ts | 68 ++++++++++++++++++++-----
 2 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/test/e2e-live/helpers/docker-helpers.ts b/test/e2e-live/helpers/docker-helpers.ts
index 567bc30..6a6f58d 100644
--- a/test/e2e-live/helpers/docker-helpers.ts
+++ b/test/e2e-live/helpers/docker-helpers.ts
@@ -381,9 +381,15 @@ export async function listContainersByNamePrefix(
     );
   }
   try {
+    // Docker's `name=` filter is a SUBSTRING match by default — `name=foo`
+    // matches any container whose name CONTAINS "foo", not just those that
+    // START with "foo". For our session-isolation guarantee (where another
+    // test run's session ID could in theory share leading hex digits with
+    // ours), we anchor the regex with `^` to force prefix semantics.
+    // Docker passes the value through to its regexp matcher, so `^` is honored.
     const { stdout } = await execFileImpl('docker', [
       'ps',
-      '--filter', `name=${namePrefix}`,
+      '--filter', `name=^${namePrefix}`,
       '--format', '{{.ID}}',
     ]);
     return stdout.split('\n').map((s) => s.trim()).filter((s) => s.length > 0);
diff --git a/test/e2e-live/helpers/tenant-fixture.ts b/test/e2e-live/helpers/tenant-fixture.ts
index 8c74d91..673f2f7 100644
--- a/test/e2e-live/helpers/tenant-fixture.ts
+++ b/test/e2e-live/helpers/tenant-fixture.ts
@@ -23,12 +23,22 @@
 import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
-import { randomUUID } from 'node:crypto';
+import { randomBytes, randomUUID } from 'node:crypto';
 import {
-  generatePrivateKey,
   getPublicKey,
   Sphere,
 } from '@unicitylabs/sphere-sdk';
+
+/**
+ * A secp256k1 private key is 32 random bytes (any value < curve order; the
+ * probability of generating an invalid one is ~2^-128, vanishingly small).
+ * Inlined so the test fixture doesn't depend on sphere-sdk's internal L1
+ * helper, which has been moved into the L1 sub-namespace and is no longer
+ * exported at the package root.
+ */
+function generatePrivateKey(): string {
+  return randomBytes(32).toString('hex');
+}
 import { createNodeProviders } from '@unicitylabs/sphere-sdk/impl/nodejs';
 
 import type {
@@ -738,17 +748,34 @@ export async function provisionTradersStaggered(
   const tasks = factories.filter((f): f is () => Promise<ProvisionedTenant> => f !== undefined);
   const concurrency = Math.min(readProvisionConcurrency(), Math.max(tasks.length, 1));
   const results: ProvisionedTenant[] = new Array(tasks.length);
+  const errors: unknown[] = [];
 
   let nextIndex = 0;
-  // Worker pool: each worker pulls the next task index and runs it. When
-  // the index pointer crosses the task list, the worker exits. With
-  // `concurrency` workers we cap simultaneous in-flight provisions.
+  // Worker pool: each worker pulls the next task index and runs it. When the
+  // index pointer crosses the task list, the worker exits. With `concurrency`
+  // workers we cap simultaneous in-flight provisions.
+  //
+  // CRITICAL: workers MUST NOT abort on first error — sequential provisioning's
+  // failure semantics are that earlier-completed tenants stay alive for the
+  // caller's afterAll to dispose. With Promise.all-style fail-fast, in-flight
+  // workers continue spawning containers AFTER the function rejects, and those
+  // containers never reach `results` so afterAll never sees them. The leak is
+  // strictly worse than sequential. We catch each task's error, record it, and
+  // let every started worker drain to completion. Once all workers settle, we
+  // dispose any tenants that did succeed (caller's afterAll won't see them
+  // through the rejected promise either) and re-throw the FIRST recorded error.
   async function worker(): Promise<void> {
     for (;;) {
       const i = nextIndex++;
       if (i >= tasks.length) return;
-      // Non-null assertion is safe: i < tasks.length guarantees defined.
-      results[i] = await tasks[i]!();
+      try {
+        // Non-null assertion is safe: i < tasks.length guarantees defined.
+        results[i] = await tasks[i]!();
+      } catch (err) {
+        errors.push(err);
+        // Continue — let other workers drain so we can clean up any
+        // late-arriving tenants below.
+      }
     }
   }
 
@@ -756,12 +783,29 @@ export async function provisionTradersStaggered(
   for (let i = 0; i < concurrency; i++) {
     workers.push(worker());
   }
-  // Promise.all rejects on the first worker error. Other workers are still
-  // running, so newly-finished provisions may need cleanup — but that
-  // cleanup is the caller's concern (afterAll always disposes whatever
-  // resolved). The original sequential implementation had the same
-  // semantics: a failure in trader N didn't dispose 0..N-1.
   await Promise.all(workers);
+
+  if (errors.length > 0) {
+    // Dispose every tenant that DID succeed before re-throwing — they are
+    // unreachable to the caller through the rejected promise.
+    for (let i = 0; i < results.length; i++) {
+      const tenant = results[i];
+      if (tenant !== undefined) {
+        try {
+          await tenant.dispose();
+        } catch {
+          // Best-effort cleanup; don't mask the original error.
+        }
+      }
+    }
+    // Re-throw the first error. If multiple workers failed, the rest are
+    // captured via `cause` so they aren't silently swallowed.
+    const primary = errors[0];
+    if (errors.length > 1 && primary instanceof Error) {
+      (primary as Error & { otherErrors?: unknown[] }).otherErrors = errors.slice(1);
+    }
+    throw primary;
+  }
   return results;
 }
 

From bef89c39f4cbfd8b6f803d3669a20163ef213cf8 Mon Sep 17 00:00:00 2001
From: Vladimir Rogojin <vrogojin@vrogojin.net>
Date: Sun, 3 May 2026 14:57:21 +0200
Subject: [PATCH 3/3] =?UTF-8?q?test(e2e-live):=20address=20PR=20#10=20revi?=
 =?UTF-8?q?ew=20warnings=20=E2=80=94=20entropy=20+=20env=20validation=20+?=
 =?UTF-8?q?=20dash-aware=20sanitizer?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three review-warning items from #10:

## W4: 32-bit → 64-bit SESSION_ID entropy

`randomBytes(4)` = 32 bits. Birthday-bound collision probability at 100
concurrent CI shards: ~1.2e-6 (small but non-zero). At 1000 shards: ~1.2e-4.
Bumping to `randomBytes(8)` = 64 bits drops collision probability to
~5e-15 even at thousands of concurrent runs — effectively impossible for
any realistic deployment. Cost: 4 additional random bytes. Trivial.

## W5: validate env-var inputs loudly, not silently

Two env vars used to silently floor invalid values:

- `TRADER_E2E_PROVISION_CONCURRENCY=0` (intent: force sequential, cc=1)
  used to fall back to DEFAULT_PROVISION_CONCURRENCY=3, surprising the
  operator. Now throws with a clear "must be >= 1, use cc=1 to force
  sequential" message.
- `VITEST_MAX_FORKS=abc` used to silently fall back to 1 via `|| 1`.
  Typos slipped through. Now throws with a "must be a positive integer"
  message. Same treatment for negative/NaN.

Both validators preserve the documented defaults when the env var is
unset or empty — no behavior change in the common case.

## W6: hex sanitizer respects dashes

`TRADER_E2E_SESSION_ID` override was sanitized via `[^0-9a-f]` strip,
silently mangling values like `ci-job-1234-abc` (CI driver tag) into
`abc`. Tighten to `[^0-9a-z-]` so dashes (which Docker container names
allow) are preserved. Length cap raised to 32 chars to match the
larger 64-bit auto-generated ID. Non-Docker-safe characters (slashes,
colons, spaces) are still stripped to prevent argv injection.

## Drive-by note

The pre-existing `mintFungibleToken` typecheck error in `src/trader/main.ts`
is fixed by PR #12. Whichever lands first wins; both touch this file.

## Test plan

- All 671 helper unit tests pass
- Typecheck clean for test/ tree (the mintFungibleToken error is upstream)
- VITEST_MAX_FORKS=abc throws "must be a positive integer"
- TRADER_E2E_PROVISION_CONCURRENCY=0 throws "must be >= 1"
- TRADER_E2E_SESSION_ID=ci-job-1234 round-trips with dashes intact
---
 test/e2e-live/helpers/session.ts        | 24 +++++++++++++++------
 test/e2e-live/helpers/tenant-fixture.ts | 17 ++++++++++++++-
 vitest.e2e-live.config.ts               | 28 ++++++++++++++++++++++---
 3 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/test/e2e-live/helpers/session.ts b/test/e2e-live/helpers/session.ts
index cced534..ca4d467 100644
--- a/test/e2e-live/helpers/session.ts
+++ b/test/e2e-live/helpers/session.ts
@@ -31,15 +31,27 @@ import { randomBytes } from 'node:crypto';
 function readSessionOverride(): string | null {
   const raw = process.env['TRADER_E2E_SESSION_ID'];
   if (typeof raw !== 'string') return null;
-  // Sanitize: lowercase, hex-only, max 16 chars. Refuse exotic characters
-  // because the value gets concatenated into Docker container names which
-  // have a strict allowed-character class ([a-zA-Z0-9_.-]).
-  const cleaned = raw.toLowerCase().replace(/[^0-9a-f]/g, '').slice(0, 16);
+  // Sanitize: lowercase, [a-z0-9-] (hex + dash), max 32 chars. We accept `-`
+  // because Docker container names allow it and CI drivers commonly tag with
+  // values like "ci-job-1234-abc" — silently stripping the dashes would mangle
+  // a meaningful identifier into an indecipherable hex blob (review feedback
+  // PR-10 W6). Stripping characters that ARE invalid for Docker names (e.g.
+  // `/`, `:`, spaces) is still required to prevent injection through the
+  // value into the docker argv.
+  const cleaned = raw.toLowerCase().replace(/[^0-9a-z-]/g, '').slice(0, 32);
   return cleaned.length > 0 ? cleaned : null;
 }
 
-/** 8-hex-char session id (32 bits ≈ 4.3 billion distinct sessions). */
-export const SESSION_ID: string = readSessionOverride() ?? randomBytes(4).toString('hex');
+/**
+ * 16-hex-char session id (64 bits ≈ 1.8e19 distinct sessions).
+ *
+ * Increased from 32 bits per PR-10 review (W4): with 100-shard CI matrices the
+ * birthday-bound collision probability at 32 bits (~1.2e-6) was small but
+ * non-zero. 64 bits drops it to ~5e-15 even at thousands of concurrent runs —
+ * effectively impossible for any realistic deployment. The cost is 4 additional
+ * random bytes; trivial.
+ */
+export const SESSION_ID: string = readSessionOverride() ?? randomBytes(8).toString('hex');
 
 /**
  * Common prefix for every container/tmp-dir name produced by this run.
diff --git a/test/e2e-live/helpers/tenant-fixture.ts b/test/e2e-live/helpers/tenant-fixture.ts
index 673f2f7..bf99502 100644
--- a/test/e2e-live/helpers/tenant-fixture.ts
+++ b/test/e2e-live/helpers/tenant-fixture.ts
@@ -717,7 +717,22 @@ function readProvisionConcurrency(): number {
   const raw = process.env['TRADER_E2E_PROVISION_CONCURRENCY'];
   if (raw === undefined || raw === '') return DEFAULT_PROVISION_CONCURRENCY;
   const n = Number.parseInt(raw, 10);
-  if (!Number.isFinite(n) || n < 1) return DEFAULT_PROVISION_CONCURRENCY;
+  // Per PR-10 review W5: silently floor invalid values to the default mask
+  // typos. A user setting `TRADER_E2E_PROVISION_CONCURRENCY=0` (intent:
+  // "force sequential, cc=1") got cc=3 instead — surprising. Reject loudly
+  // for invalid forms so the operator notices the typo. We DO honor cc=1
+  // explicitly as "sequential" and cc=0 is now an explicit error.
+  if (!Number.isFinite(n) || Number.isNaN(n)) {
+    throw new Error(
+      `Invalid TRADER_E2E_PROVISION_CONCURRENCY="${raw}": must be a positive integer.`,
+    );
+  }
+  if (n < 1) {
+    throw new Error(
+      `Invalid TRADER_E2E_PROVISION_CONCURRENCY="${raw}" (parsed as ${n}): must be >= 1. ` +
+        `Use TRADER_E2E_PROVISION_CONCURRENCY=1 to force sequential provisioning.`,
+    );
+  }
   return n;
 }
 
diff --git a/vitest.e2e-live.config.ts b/vitest.e2e-live.config.ts
index f0e5f9e..7ddd649 100644
--- a/vitest.e2e-live.config.ts
+++ b/vitest.e2e-live.config.ts
@@ -29,13 +29,35 @@ import { defineConfig } from 'vitest/config';
  * shared testnet Nostr relay is the bottleneck. Don't raise this above
  * what the relay can handle (empirically 2-3 today).
  *
+ * Note: each fork spawns its own controller wallet via `Sphere.init`,
+ * which performs a relay handshake. With N forks you do N concurrent
+ * Sphere.init calls against the same relay during startup. If the relay
+ * sustains 2-3 concurrent inits today, plan accordingly.
+ *
  * The session-isolation work in test/e2e-live/helpers/session.ts is what
  * makes cross-fork (and cross-process) parallelism safe — see that file
  * for the per-resource analysis.
+ *
+ * Per PR-10 review W5: validate explicitly rather than silently floor on
+ * NaN/0/negative — typos used to be hidden by `|| 1`, now they fail loudly.
  */
-const MAX_FORKS = process.env['VITEST_MAX_FORKS']
-  ? Math.max(1, Number.parseInt(process.env['VITEST_MAX_FORKS'], 10) || 1)
-  : 1;
+function readMaxForks(): number {
+  const raw = process.env['VITEST_MAX_FORKS'];
+  if (raw === undefined || raw === '') return 1;
+  const n = Number.parseInt(raw, 10);
+  if (!Number.isFinite(n) || Number.isNaN(n)) {
+    throw new Error(
+      `Invalid VITEST_MAX_FORKS="${raw}": must be a positive integer.`,
+    );
+  }
+  if (n < 1) {
+    throw new Error(
+      `Invalid VITEST_MAX_FORKS="${raw}" (parsed as ${n}): must be >= 1.`,
+    );
+  }
+  return n;
+}
+const MAX_FORKS = readMaxForks();
 
 export default defineConfig({
   test: {