fix(browser): caller systemMetricsSampler no longer duplicates producer-owned traces

docs/design/system-traces-expansion.md

@@ -501,7 +501,7 @@ shared `PageLifecycleTracker`, delete the legacy samplers, then ST1+ traces as p
 **Status (2026-10): shipped.** ST0′ (#55), the browser trackers (#62), `PageLifecycleTracker` (#63) and the node legacy-sampler
 deletion (#65; the browser sampler went with #62) are merged, so the bare `sample()` adapter no longer coexists with the producers on browser or node: the legacy
 browser sampler (which also emitted `battery`/`charging`, the duplicate rows an interim reading of this section implied) is
-deleted and `systemMetricsSampler` survives only as an explicit caller override that REPLACES the heap, connection, orientation, `app_state` and `fps` sensor traces; the `battery` producer still runs alongside it (`browser/src/launch.ts:925-933`), so a sampler that also emits `battery`/`charging` would duplicate those rows. This is a code-level gap against the one-writer rule (#132) and is NOT fixed (the override at `packages/browser/src/launch.ts:1096-1108` neither filters those names nor suppresses the producer); it is tracked as a code follow-up, outside this docs PR.
+deleted and `systemMetricsSampler` survives only as an explicit caller override that REPLACES the heap, connection, orientation, `app_state` and `fps` sensor traces; the `battery` producer still runs alongside it (`browser/src/launch.ts`, the `createSystemTracesProvider` wiring), so a sampler that also emits `battery`/`charging` would have duplicated those rows. Fixed (#154): the producers that keep running alongside the override own their names (`battery`, `charging`, `battery_charging_time_s`, `battery_discharging_time_s`, `route`, `view`, `url` when captured, and `capture` when replay is on) and `launch` drops sampler rows under them (`withoutNames`/`producerOwnedNames`, `packages/browser/src/launch.ts`), so there is one writer per name; the sampler's other rows are kept.
 
 The domain-tracker follow-on (§15) is tracked in §15.5. The node `ProcessStatsTracker` that §14.5 lists for ST0′ was replaced by the node `ProcessTracker` in TR8 part 1 (#87): its files, tests and `@bugsee/node` exports (`ProcessStatsTracker`, `ProcessStats`) are gone, and `packages/trackers/src/node/` is no longer a placeholder. The Electron trackers (ST6 above) shipped as TR9 (#90, #91). The per-signal browser trackers that §14.5 originally listed as remaining (`MemoryTracker`, `OrientationTracker`, `FpsTracker`, `ScreenSizeTracker`, `MainThreadLagTracker`) were folded into the domain trackers by TR2 (#82) and TR5 (#83) and their files are deleted: `packages/browser/src/trackers/` no longer exists on `main`.
 

packages/browser/src/launch.test.ts

@@ -1356,6 +1356,55 @@ describe('launch', () => {
     );
   });
 
+  it('a caller sampler returning battery/charging does not duplicate the battery producer (one writer per name)', async () => {
+    vi.stubGlobal('navigator', { getBattery: () => Promise.resolve(batteryManager()) });
+    const store = memStore();
+    await launchTracked(
+      'tok',
+      baseOptions({
+        captureStore: store,
+        systemMetricsSampler: () => [
+          { name: 'ram_js_heap_used', value: 42 },
+          { name: 'battery', value: 7 },
+          { name: 'charging', value: { state: 1 } },
+          { name: 'battery_charging_time_s', value: 111 },
+          { name: 'battery_discharging_time_s', value: 222 },
+          { name: 'route', value: 'sampler-route' },
+          { name: 'view', value: 'sampler-view' },
+          { name: 'url', value: 'sampler-url' },
+        ],
+      }),
+    );
+    await new Promise((r) => setTimeout(r, 0));
+    const traces = ((await drain(store, 'traces.system')) ?? []).map(
+      (e) => e.data as { name: string; value: unknown },
+    );
+    // the producer owns the names: the sampler's rows are dropped, its other rows are kept
+    expect(traces.filter((t) => t.name === 'battery').map((t) => t.value)).toEqual([50]);
+    expect(traces.filter((t) => t.name === 'charging').map((t) => t.value)).toEqual([{ state: 2 }]);
+    expect(traces).toContainEqual(expect.objectContaining({ name: 'ram_js_heap_used', value: 42 }));
+    const values = traces.map((t) => t.value);
+    for (const leaked of [111, 222, 'sampler-route', 'sampler-view', 'sampler-url']) {
+      expect(values).not.toContain(leaked);
+    }
+  });
+
+  it('a caller sampler may still write url when URL capture is off', async () => {
+    const store = memStore();
+    await launchTracked(
+      'tok',
+      baseOptions({
+        captureStore: store,
+        captureUrl: false,
+        systemMetricsSampler: () => [{ name: 'url', value: 'sampler-url' }],
+      }),
+    );
+    const traces = ((await drain(store, 'traces.system')) ?? []).map(
+      (e) => e.data as { name: string; value: unknown },
+    );
+    expect(traces).toContainEqual(expect.objectContaining({ name: 'url', value: 'sampler-url' }));
+  });
+
   it('a battery manager that throws after getBattery resolves reaches options.onError', async () => {
     const getBattery = vi.fn(() =>
       Promise.resolve({
@@ -3909,6 +3958,42 @@ describe('launch — session replay (lazy)', () => {
     await vi.waitFor(() => expect(registerReplay).toHaveBeenCalledTimes(1));
   });
 
+  it.each([
+    { replay: undefined, kept: false },
+    { replay: false, kept: true },
+  ])('a caller sampler row named `capture` is dropped only while the capture producer exists (replay=$replay)', async ({
+    replay,
+    kept,
+  }) => {
+    const store = memStore();
+    // A real recorder makes the producer's own `capture` row appear (`active`); `undefined` would leave it unwritten.
+    if (replay === undefined) registerReplay.mockReturnValueOnce({ stop: vi.fn() });
+    await launchTracked(
+      'tok',
+      domReplayOptions({
+        captureStore: store,
+        ...(replay === undefined ? {} : { replay }),
+        systemMetricsSampler: () => [{ name: 'capture', value: 'sampler-capture' }],
+      }),
+    );
+    if (replay === undefined) {
+      await vi.waitFor(() => expect(registerReplay).toHaveBeenCalledTimes(1));
+      await vi.waitFor(async () => {
+        const rows = ((await drain(store, 'traces.system')) ?? []).map(
+          (e) => e.data as { value: unknown },
+        );
+        // the producer's row survives the filter, and the caller's same-named row does not
+        expect(rows.some((r) => (r.value as { state?: string } | null)?.state === 'active')).toBe(
+          true,
+        );
+      });
+    }
+    const values = ((await drain(store, 'traces.system')) ?? []).map(
+      (e) => (e.data as { value: unknown }).value,
+    );
+    expect(values.includes('sampler-capture')).toBe(kept);
+  });
+
   it('the DEFAULT path resolves to FAIL-CLOSED masking (mask all text/inputs, block all media)', async () => {
     // The default now applies to EVERY integration that does not opt out, so "fail-closed" has to hold on
     // the default path and not merely on the opted-in one. Asserting the forwarded options object is `{}`

packages/browser/src/launch.ts

@@ -22,6 +22,7 @@ import {
   type NetworkCapture,
   type TraceSample,
   type TraceSuppression,
+  withoutTraceNames,
 } from '@bugsee/capture';
 import {
   asInternalClient,
@@ -82,7 +83,7 @@ import {
   type TriggerPipeline,
   type UploadResult,
 } from '@bugsee/core';
-import { BugseeOption, type EnvironmentEnvelope } from '@bugsee/protocol';
+import { BugseeOption, type EnvironmentEnvelope, TraceName } from '@bugsee/protocol';
 import { applicationTrackerFor } from '@bugsee/trackers/browser';
 import { reportGuarded } from '@bugsee/util';
 import { BROWSER_APP_VERSION_HINT, metaAppIdentity, warnToConsole } from './app-identity';
@@ -568,7 +569,7 @@ export interface BugseeLaunchOptions {
   systemProbe?: BrowserProbe;
   /**
    * LEGACY polled system-traces override. It REPLACES the default heap/connection/orientation traces (as on
-   * Node); battery stays a tracker-backed producer. The defaults are producers, not a sampler.
+   * Node); battery stays a tracker-backed producer that (with `url`/`route`/`view`) owns its trace names; sampler rows under them are dropped. The defaults are producers, not a sampler.
    */
   systemMetricsSampler?: () => readonly TraceSample[];
   /** Durable bundle store override (crash recovery across reloads). Default none (until B5). */
@@ -662,6 +663,20 @@ function reportInvalidHostOwned(onError: ((error: unknown) => void) | undefined)
   );
 }
 
+/** Names the producers that run alongside a caller sampler (`sensors: false`) own: battery x4, route, view, url when built, and capture when replay is on. */
+function producerOwnedNames(withUrl: boolean, withCapture: boolean): ReadonlySet<string> {
+  return new Set<string>([
+    TraceName.BATTERY,
+    TraceName.CHARGING,
+    TraceName.BATTERY_CHARGING_TIME_S,
+    TraceName.BATTERY_DISCHARGING_TIME_S,
+    TraceName.ROUTE,
+    TraceName.VIEW,
+    ...(withUrl ? [TraceName.URL] : []),
+    ...(withCapture ? [TraceName.CAPTURE] : []),
+  ]);
+}
+
 // The public composition root's lifecycle shell: publishes Launching while the (async) composition is in
 // flight, Launched once it has built a client, and falls back to Stopped if it throws. A repeat launch
 // (a client already owns the slot) leaves the state alone.
@@ -1093,11 +1108,19 @@ async function buildLaunch(
   if (diskPressure !== undefined && resolved.isEnabled(BugseeOption.CaptureDiskSpace)) {
     captureState?.bindPressure(diskPressure); // the disk STATES are telemetry: behind the opt-in, the protection is not
   }
+  const captureUrl = resolved.options.get<boolean>(CAPTURE_URL_OPTION_KEY, true);
   client.addCaptureProvider(
     createSystemTracesProvider({
-      // A `systemMetricsSampler` REPLACES the sensor traces (heap, connection, orientation, app_state, fps), as on Node; battery still runs.
+      // A `systemMetricsSampler` REPLACES the sensor traces (heap, connection, orientation, app_state, fps), as on Node; battery still
+      // runs alongside it and OWNS its names (battery, charging, the two battery times, route, view, url when captured, capture when replay is on): a sampler row
+      // under those names is dropped (one writer per trace name, #154).
       ...(options.systemMetricsSampler !== undefined
-        ? { sample: options.systemMetricsSampler }
+        ? {
+            sample: withoutTraceNames(
+              options.systemMetricsSampler,
+              producerOwnedNames(captureUrl, captureState !== undefined),
+            ),
+          }
         : {}),
       ...(suppression !== undefined ? { suppression } : {}),
       producers: [
@@ -1108,7 +1131,7 @@ async function buildLaunch(
             ...(options.systemMetricsSampler !== undefined ? { sensors: false } : {}),
             captureUserIdle: resolved.options.get<boolean>(CAPTURE_USER_IDLE_OPTION_KEY, false),
             // The page's own sanitized address: written from the navigation source, not from the performance extension.
-            ...(resolved.options.get<boolean>(CAPTURE_URL_OPTION_KEY, true)
+            ...(captureUrl
               ? withUrl(
                   urlProducerDepsFor(win, {
                     ...(carrier !== undefined ? { carrier } : {}),

packages/electron/src/host-owned-traces.e2e.test.ts

@@ -271,14 +271,15 @@ describe('host-owned traces: Electron renderers write none of the names main own
       systemMetricsSampler: () => [
         { name: 'capture', value: { state: 'active' } },
         { name: 'displays', value: [{ id: 0, width: 800, height: 600 }] },
-        { name: 'battery', value: 42 },
+        // `battery` is owned by the renderer's own producer (a sampler row under it is dropped, #154), so use a sampler-only name
+        { name: 'app_state', value: 42 },
       ],
     });
     r.tick();
     await client.logException(new Error('report'));
     const traces = tracesOf(puts[0] as Uint8Array);
     expect(traces.some((t) => t.name === 'capture' || t.name === 'displays')).toBe(false);
-    expect(traces.filter((t) => t.name === 'battery').map((t) => t.value)).toContain(42); // unlisted still flows
+    expect(traces.filter((t) => t.name === 'app_state').map((t) => t.value)).toContain(42); // unlisted still flows
     await client.stop();
   });