Merge branch 'unstable' into test-audit/auth-service-coverage-12553948378284610464

CONTRIBUTING.md

@@ -41,6 +41,8 @@ Automated agents contributing to this repository should follow these rules:
 - **Commit Messages**: Use the convention `type(scope): description (agent)` (e.g., `fix(ai): formatting (agent)` or `docs(ai): update quickstart (agent)`).
 - **Reference Issues**: Link to relevant issues in the PR description.
 - **Review AGENTS.md**: Always consult `AGENTS.md` for specific architectural guidelines (like URL-first playback, token-first design system) and constraints before starting work.
+- **Review KNOWN_ISSUES.md**: Before starting development or debugging, review `KNOWN_ISSUES.md` to avoid investigating pre-existing failures or limitations.
+- **Dependency Upgrades**: Major or risky dependency upgrades must be documented by creating a Markdown file in the `issues/` directory (e.g., `issues/upgrade-<pkg>.md`) with a plan and risk assessment, rather than opening an immediate PR.
 - **Efficiency**: Prefer running targeted or sharded tests (e.g., `npm run test:unit:shard1`, `shard2`, or `shard3`) to conserve resources during iteration.
 
 ## Submitting a Pull Request

README.md

@@ -181,6 +181,8 @@ To run **bitvid** locally:
    npm run lint
    ```
 
+   _Tip: Use `npm run test:unit:shard1` (or `shard2`/`shard3`) for faster local feedback._
+
 #### Quick Reference
 
 - **Run unit tests**: `npm run test:unit`

test-audit-report-2026-02-12.md

@@ -0,0 +1,35 @@
+# Test Audit Report - 2026-02-12
+
+## Summary
+The unit test suite passed successfully (680 tests). A critical issue where `tests/auth-service.test.mjs` was skipped (resulting in 10% coverage for `authService.js`) was identified and fixed, raising coverage to 62%. Several critical production files remain with low coverage.
+
+## Test Run Details
+*   **Command:** `node --import ./tests/test-helpers/setup-localstorage.mjs --test --experimental-test-coverage tests/**/*.test.mjs`
+*   **Result:** 680 passed, 0 failed.
+*   **Duration:** ~42s
+
+## Critical Coverage Gaps
+The following critical files have low coverage (< 50%):
+
+| File | Line Coverage | Function Coverage | Impact |
+|------|--------------|-------------------|--------|
+| `js/webtorrent.js` | 12.25% | 4.00% | **Critical** (Playback core) |
+| `js/userBlocks.js` | 18.92% | 26.47% | **High** (Moderation) |
+| `js/nostr/nip46Client.js` | 27.42% | 44.59% | **High** (Remote Signing) |
+| `js/views/VideoListView.js` | 48.81% | 36.04% | **Medium** (Main Feed UI) |
+
+## Fixed Issues
+1.  **Auth Service Tests Skipped:** `tests/auth-service.test.mjs` was using raw async blocks without `node:test` imports, causing the runner to skip it.
+    *   **Fix:** Refactored to use `describe`/`test` from `node:test`.
+    *   **Result:** `js/services/authService.js` coverage improved from 10% to 62%.
+2.  **Console Noise:** Removed a leftover `console.log` in `tests/watchHistory/watch-history-telemetry.test.mjs`.
+
+## Static Analysis Findings
+*   **Time Dependency:** Extensive use of `setTimeout` found in `tests/nostr/client.test.mjs`, `tests/moderation/video-card.test.mjs`, and others. These are potential sources of flakiness.
+*   **Skipped Tests:** `tests/nostr/sessionActor.test.mjs` skips a test when WebCrypto is unavailable.
+*   **Network Calls:** No unmocked external network calls detected in unit tests (mocks appear to be used).
+
+## Recommendations
+1.  **Prioritize Webtorrent Tests:** Add unit tests for `js/webtorrent.js` to cover the torrent client wrapper logic.
+2.  **Moderation Coverage:** Expand `tests/user-blocks.test.mjs` (if it exists) or create it to cover `js/userBlocks.js`.
+3.  **Refactor Timeouts:** Replace `await new Promise(r => setTimeout(r, N))` with deterministic `waitFor` helpers in `tests/nostr/client.test.mjs` to reduce flakiness risk.

tests/auth-service.test.mjs

@@ -2,6 +2,7 @@
 
 import "./test-helpers/setup-localstorage.mjs";
 import assert from "node:assert/strict";
+import { test, describe, beforeEach } from "node:test";
 
 import AuthService from "../js/services/authService.js";
 import {
@@ -70,162 +71,134 @@ function resetState() {
   }
 }
 
-await (async () => {
-  resetState();
-  setPubkey(SAMPLE_PUBKEY);
+describe("AuthService", () => {
+  beforeEach(resetState);
 
-  const calls = [];
-  let resolveBlocks;
+  test("applyPostLoginState success flow", async () => {
+    setPubkey(SAMPLE_PUBKEY);
 
-  const service = new AuthService({
-    userBlocks: {
-      loadBlocks: () => {
-        calls.push("blocks:start");
-        return new Promise((resolve) => {
-          resolveBlocks = resolve;
-        });
+    const calls = [];
+    let resolveBlocks;
+
+    const service = new AuthService({
+      userBlocks: {
+        loadBlocks: () => {
+          calls.push("blocks:start");
+          return new Promise((resolve) => {
+            resolveBlocks = resolve;
+          });
+        },
       },
-    },
-    relayManager: {
-      loadRelayList: () => {
-        calls.push("relays:start");
-        return Promise.resolve("relays");
+      relayManager: {
+        loadRelayList: () => {
+          calls.push("relays:start");
+          return Promise.resolve("relays");
+        },
       },
-    },
+    });
+
+    service.loadOwnProfile = () => {
+      calls.push("profile:start");
+      return Promise.resolve({ name: "Test User" });
+    };
+
+    const promise = service.applyPostLoginState();
+
+    await new Promise((resolve) => setTimeout(resolve, 10));
+
+    assert.deepEqual(calls, ["relays:start", "blocks:start", "profile:start"]);
+
+    resolveBlocks();
+
+    const result = await promise;
+    const detail = result.detail;
+    await result.completionPromise;
+
+    assert.equal(detail.pubkey, SAMPLE_PUBKEY);
+    assert.equal(detail.blocksLoaded, true);
+    assert.equal(detail.relaysLoaded, true);
+    assert.deepEqual(detail.profile, { name: "Test User" });
   });
 
-  service.loadOwnProfile = () => {
-    calls.push("profile:start");
-    return Promise.resolve({ name: "Test User" });
-  };
+  test("applyPostLoginState with deferBlocks: true", async () => {
+    setPubkey(SAMPLE_PUBKEY);
 
-  const promise = service.applyPostLoginState();
+    const calls = [];
+    // We leave resolveBlocks undefined/pending forever to simulate slow/hanging blocks
+    let resolveBlocks;
 
-  await new Promise((resolve) => setTimeout(resolve, 10));
-
-  assert.deepEqual(calls, ["relays:start", "blocks:start", "profile:start"]);
-
-  resolveBlocks();
-
-  const result = await promise;
-  const detail = result.detail;
-  await result.completionPromise;
-
-  assert.equal(detail.pubkey, SAMPLE_PUBKEY);
-  assert.equal(detail.blocksLoaded, true);
-  assert.equal(detail.relaysLoaded, true);
-  assert.deepEqual(detail.profile, { name: "Test User" });
-})();
-
-await (async () => {
-  resetState();
-  setPubkey(SAMPLE_PUBKEY);
-
-  const calls = [];
-  // We leave resolveBlocks undefined/pending forever to simulate slow/hanging blocks
-  let resolveBlocks;
-
-  const service = new AuthService({
-    userBlocks: {
-      loadBlocks: () => {
-        calls.push("blocks:start");
-        return new Promise((resolve) => {
-          resolveBlocks = resolve;
-        });
+    const service = new AuthService({
+      userBlocks: {
+        loadBlocks: () => {
+          calls.push("blocks:start");
+          return new Promise((resolve) => {
+            resolveBlocks = resolve;
+          });
+        },
       },
-    },
-    relayManager: {
-      loadRelayList: () => {
-        calls.push("relays:start");
-        return Promise.resolve("relays");
+      relayManager: {
+        loadRelayList: () => {
+          calls.push("relays:start");
+          return Promise.resolve("relays");
+        },
       },
-    },
+    });
+
+    service.loadOwnProfile = () => {
+      calls.push("profile:start");
+      return Promise.resolve({ name: "Test User" });
+    };
+
+    // Test deferBlocks: true
+    const promise = service.applyPostLoginState({ deferBlocks: true });
+
+    await new Promise((resolve) => setTimeout(resolve, 10));
+
+    // Blocks should NOT be started if deferred
+    // Wait, deferBlocks just means it's not awaited in completionPromise?
+    // No, checking code:
+    // if (!deferBlocks) { const blocksOperation = this.createBlocksLoadOperation... }
+    // So it shouldn't even be called/pushed to concurrentOps.
+
+    assert.deepEqual(calls, ["relays:start", "profile:start"]);
+
+    const result = await promise;
+    // This should resolve even if blocks are not loaded
+    await result.completionPromise;
+
+    assert.equal(result.detail.blocksLoaded, null);
+    assert.equal(result.detail.relaysLoaded, true);
   });
 
-  service.loadOwnProfile = () => {
-    calls.push("profile:start");
-    return Promise.resolve({ name: "Test User" });
-  };
+  test("applyPostLoginState partial failure", async () => {
+    setPubkey(SAMPLE_PUBKEY);
 
-  // Test deferBlocks: true
-  const promise = service.applyPostLoginState({ deferBlocks: true });
-
-  await new Promise((resolve) => setTimeout(resolve, 10));
-
-  // Blocks should NOT be started if deferred
-  // Wait, deferBlocks just means it's not awaited in completionPromise?
-  // No, checking code:
-  // if (!deferBlocks) { const blocksOperation = this.createBlocksLoadOperation... }
-  // So it shouldn't even be called/pushed to concurrentOps.
-
-  assert.deepEqual(calls, ["relays:start", "profile:start"]);
-
-  const result = await promise;
-  // This should resolve even if blocks are not loaded
-  await result.completionPromise;
-
-  assert.equal(result.detail.blocksLoaded, null);
-  assert.equal(result.detail.relaysLoaded, true);
-})();
-
-await (async () => {
-  resetState();
-  setPubkey(SAMPLE_PUBKEY);
-
-  const logs = [];
-  const service = new AuthService({
-    userBlocks: {
-      loadBlocks: () => {
-        throw new Error("blocks failed synchronously");
+    const logs = [];
+    const service = new AuthService({
+      userBlocks: {
+        loadBlocks: () => {
+          throw new Error("blocks failed synchronously");
+        },
       },
-    },
-    relayManager: {
-      loadRelayList: () => Promise.reject(new Error("relays failed")),
-    },
-  });
+      relayManager: {
+        loadRelayList: () => Promise.reject(new Error("relays failed")),
+      },
+    });
 
-  service.log = (message, error) => {
-    logs.push({ message, error });
-  };
+    service.log = (message, error) => {
+      logs.push({ message, error });
+    };
 
-  service.loadOwnProfile = () => Promise.reject(new Error("profile failed"));
+    service.loadOwnProfile = () => Promise.reject(new Error("profile failed"));
 
-  const result = service.applyPostLoginState();
-  const detail = result.detail;
-  await result.completionPromise;
+    const result = service.applyPostLoginState();
+    const detail = result.detail;
+    await result.completionPromise;
 
-  assert.equal(detail.pubkey, SAMPLE_PUBKEY);
-  assert.equal(detail.blocksLoaded, false);
-  assert.equal(detail.relaysLoaded, false);
-  assert.deepEqual(detail.profile, {
-    name: "Unknown",
-    picture: "assets/svg/default-profile.svg",
-    about: "",
-    website: "",
-    banner: "",
-    lud16: "",
-    lud06: "",
-  });
-
-  assert.equal(logs.length, 3);
-  assert(logs.some(({ message }) => message.includes("block list")));
-  assert(logs.some(({ message }) => message.includes("relay list")));
-  assert(logs.some(({ message }) => message.includes("own profile")));
-})();
-
-await (async () => {
-  resetState();
-
-  const service = new AuthService();
-  const result = service.applyPostLoginState();
-  const detail = result.detail;
-  await result.completionPromise;
-
-  assert.deepEqual(detail, {
-    pubkey: null,
-    blocksLoaded: false,
-    relaysLoaded: false,
-    profile: {
+    assert.equal(detail.pubkey, SAMPLE_PUBKEY);
+    assert.equal(detail.blocksLoaded, false);
+    assert.equal(detail.relaysLoaded, false);
+    assert.deepEqual(detail.profile, {
       name: "Unknown",
       picture: "assets/svg/default-profile.svg",
       about: "",
@@ -233,170 +206,176 @@ await (async () => {
       banner: "",
       lud16: "",
       lud06: "",
-    },
-  });
-})();
+    });
 
-await (async () => {
-  resetState();
-
-  const service = new AuthService();
-  const postLogin = {
-    pubkey: SAMPLE_PUBKEY,
-    blocksLoaded: true,
-    relaysLoaded: false,
-    profile: { name: "Stub" },
-  };
-
-  service.applyPostLoginState = () => ({
-    detail: postLogin,
-    completionPromise: Promise.resolve(postLogin),
+    assert.equal(logs.length, 3);
+    assert(logs.some(({ message }) => message.includes("block list")));
+    assert(logs.some(({ message }) => message.includes("relay list")));
+    assert(logs.some(({ message }) => message.includes("own profile")));
   });
 
-  const detail = await service.login(SAMPLE_PUBKEY);
+  test("applyPostLoginState without pubkey", async () => {
+    const service = new AuthService();
+    const result = service.applyPostLoginState();
+    const detail = result.detail;
+    await result.completionPromise;
 
-  await detail.postLoginPromise;
-
-  assert.equal(detail.postLogin, postLogin);
-  assert.deepEqual(detail.postLogin, postLogin);
-})();
-
-await (async () => {
-  resetState();
-
-  const adminCheckCalls = [];
-  const service = new AuthService({
-    accessControl: {
-      isLockdownActive: () => true,
-      isAdminEditor: (npub) => {
-        adminCheckCalls.push(npub);
-        return false;
+    assert.deepEqual(detail, {
+      pubkey: null,
+      blocksLoaded: false,
+      relaysLoaded: false,
+      profile: {
+        name: "Unknown",
+        picture: "assets/svg/default-profile.svg",
+        about: "",
+        website: "",
+        banner: "",
+        lud16: "",
+        lud06: "",
       },
-    },
+    });
   });
 
-  let error;
-  try {
-    await service.login(SAMPLE_PUBKEY);
-  } catch (err) {
-    error = err;
-  }
-
-  assert(error instanceof Error);
-  assert.equal(error.code, "site-lockdown");
-  assert.match(error.message, /locked down/i);
-  assert.equal(adminCheckCalls.length, 1);
-  assert.equal(typeof adminCheckCalls[0], "string");
-  assert(adminCheckCalls[0].length > 0);
-})();
-
-await (async () => {
-  resetState();
-
-  const service = new AuthService({
-    accessControl: {
-      isLockdownActive: () => true,
-      isAdminEditor: () => true,
-    },
-  });
-
-  const detail = await service.login(SAMPLE_PUBKEY);
-
-  assert.ok(detail);
-  assert.equal(detail.pubkey, SAMPLE_PUBKEY);
-})();
-
-// === Tests for loadOwnProfile logic ===
-
-await (async () => {
-  resetState();
-  const PUBKEY = SAMPLE_PUBKEY;
-
-  // Test Fast Relay Success
-  {
-    const mockNostrClient = {
-      relays: ["wss://fast1.com", "wss://fast2.com", "wss://slow.com"],
-      pool: {
-        list: async (relays) => {
-          const url = relays[0];
-          if (url === "wss://fast1.com") {
-            return [{
-              pubkey: PUBKEY,
-              created_at: 100,
-              content: JSON.stringify({ name: "Fast User" })
-            }];
-          }
-          return new Promise(() => {}); // Hang others
-        }
-      },
-      handleEvent: () => {}
+  test("login mocks postLogin flow", async () => {
+    const service = new AuthService();
+    const postLogin = {
+      pubkey: SAMPLE_PUBKEY,
+      blocksLoaded: true,
+      relaysLoaded: false,
+      profile: { name: "Stub" },
     };
 
-    const service = new AuthService({ nostrClient: mockNostrClient });
-    // Override setProfileCacheEntry to avoid side effects or errors from real implementation if not needed,
-    // but here we just let it run. However, the service needs nostrClient.pool
+    service.applyPostLoginState = () => ({
+      detail: postLogin,
+      completionPromise: Promise.resolve(postLogin),
+    });
 
-    // We need to make sure setProfileCacheEntry doesn't fail.
-    // It imports from cache.js which uses profileCache.
-    // profileCache is imported from state/profileCache.js.
+    const detail = await service.login(SAMPLE_PUBKEY);
 
-    // The previous tests didn't seem to mock setProfileCacheEntry.
+    await detail.postLoginPromise;
 
-    const profile = await service.loadOwnProfile(PUBKEY);
-    assert.equal(profile.name, "Fast User");
-  }
+    assert.equal(detail.postLogin, postLogin);
+    assert.deepEqual(detail.postLogin, postLogin);
+  });
 
-  // Test All Relays Fail
-  {
-    const mockNostrClient = {
-      relays: ["wss://fail1.com", "wss://fail2.com"],
-      pool: {
-        list: async () => {
-          throw new Error("Failed");
-        }
+  test("login fails on lockdown", async () => {
+    const adminCheckCalls = [];
+    const service = new AuthService({
+      accessControl: {
+        isLockdownActive: () => true,
+        isAdminEditor: (npub) => {
+          adminCheckCalls.push(npub);
+          return false;
+        },
       },
-      handleEvent: () => {}
-    };
+    });
 
-    const service = new AuthService({ nostrClient: mockNostrClient });
-    const profile = await service.loadOwnProfile(PUBKEY);
-    assert.equal(profile.name, "Unknown");
-  }
+    let error;
+    try {
+      await service.login(SAMPLE_PUBKEY);
+    } catch (err) {
+      error = err;
+    }
 
-  // Test Timeout Fallback
-  {
-    let backgroundResolve;
-    const backgroundPromise = new Promise(r => backgroundResolve = r);
+    assert(error instanceof Error);
+    assert.equal(error.code, "site-lockdown");
+    assert.match(error.message, /locked down/i);
+    assert.equal(adminCheckCalls.length, 1);
+    assert.equal(typeof adminCheckCalls[0], "string");
+    assert(adminCheckCalls[0].length > 0);
+  });
 
-    const mockNostrClient = {
-        relays: ["wss://fast-fail.com", "wss://fast-fail2.com", "wss://background-ok.com", "wss://background-ok-2.com"],
+  test("login succeeds for admin on lockdown", async () => {
+    const service = new AuthService({
+      accessControl: {
+        isLockdownActive: () => true,
+        isAdminEditor: () => true,
+      },
+    });
+
+    const detail = await service.login(SAMPLE_PUBKEY);
+
+    assert.ok(detail);
+    assert.equal(detail.pubkey, SAMPLE_PUBKEY);
+  });
+
+  describe("loadOwnProfile", () => {
+    const PUBKEY = SAMPLE_PUBKEY;
+
+    test("Fast Relay Success", async () => {
+      const mockNostrClient = {
+        relays: ["wss://fast1.com", "wss://fast2.com", "wss://slow.com"],
         pool: {
-            list: async (relays) => {
-                const url = relays[0];
-                if (url.includes("fast")) {
-                    // Simulate timeout or error
-                    throw new Error("Fast fail");
-                }
-                if (url.includes("background")) {
-                    await backgroundPromise;
-                    return [{
-                        pubkey: PUBKEY,
-                        created_at: 200,
-                        content: JSON.stringify({ name: "Background User" })
-                    }];
-                }
-                return [];
+          list: async (relays) => {
+            const url = relays[0];
+            if (url === "wss://fast1.com") {
+              return [{
+                pubkey: PUBKEY,
+                created_at: 100,
+                content: JSON.stringify({ name: "Fast User" })
+              }];
             }
+            return new Promise(() => {}); // Hang others
+          }
         },
         handleEvent: () => {}
-    };
+      };
 
-    const service = new AuthService({ nostrClient: mockNostrClient });
+      const service = new AuthService({ nostrClient: mockNostrClient });
+      const profile = await service.loadOwnProfile(PUBKEY);
+      assert.equal(profile.name, "Fast User");
+    });
 
-    const profile = await service.loadOwnProfile(PUBKEY);
-    assert.equal(profile.name, "Unknown");
+    test("All Relays Fail", async () => {
+      const mockNostrClient = {
+        relays: ["wss://fail1.com", "wss://fail2.com"],
+        pool: {
+          list: async () => {
+            throw new Error("Failed");
+          }
+        },
+        handleEvent: () => {}
+      };
 
-    // We resolve background promise to let background logic finish eventually (though test finishes here)
-    backgroundResolve();
-  }
-})();
+      const service = new AuthService({ nostrClient: mockNostrClient });
+      const profile = await service.loadOwnProfile(PUBKEY);
+      assert.equal(profile.name, "Unknown");
+    });
+
+    test("Timeout Fallback", async () => {
+      let backgroundResolve;
+      const backgroundPromise = new Promise(r => backgroundResolve = r);
+
+      const mockNostrClient = {
+          relays: ["wss://fast-fail.com", "wss://fast-fail2.com", "wss://background-ok.com", "wss://background-ok-2.com"],
+          pool: {
+              list: async (relays) => {
+                  const url = relays[0];
+                  if (url.includes("fast")) {
+                      // Simulate timeout or error
+                      throw new Error("Fast fail");
+                  }
+                  if (url.includes("background")) {
+                      await backgroundPromise;
+                      return [{
+                          pubkey: PUBKEY,
+                          created_at: 200,
+                          content: JSON.stringify({ name: "Background User" })
+                      }];
+                  }
+                  return [];
+              }
+          },
+          handleEvent: () => {}
+      };
+
+      const service = new AuthService({ nostrClient: mockNostrClient });
+
+      const profile = await service.loadOwnProfile(PUBKEY);
+      assert.equal(profile.name, "Unknown");
+
+      // We resolve background promise to let background logic finish eventually (though test finishes here)
+      backgroundResolve();
+    });
+  });
+});

tests/watchHistory/watch-history-telemetry.test.mjs

@@ -221,4 +221,3 @@ await testPreparePlaybackLoggingPublishesOnce();
 await testPreparePlaybackLoggingIgnoresMetadata();
 await testCancelPlaybackLoggingFlushes();
 
-console.log("watch-history-telemetry tests passed");