From e32298cc8985b1ee80c1017c9fe66473a22d1131 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:01:20 -0400 Subject: [PATCH 01/27] uploads: locked-storage file picks unlock inline instead of dead-ending (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three paths (upload video pick, upload thumbnail pick, edit "Replace file") now route through the shared signer gate — silent kept-unlocked restore, else ONE passphrase prompt — then unlock storage and continue the same pick. Cancel/bad-passphrase abort quietly; a refused thumbnail pick restores the picker. Unlock glue extracted to uploadModalStorageUnlock.js (file-size budget); ensureSigner threaded via initUploadModal/initEditModal. Tests: tests/upload-edit-storage-unlock-gate.test.mjs (12). Co-Authored-By: Claude Fable 5 --- js/ui/components/EditModal.js | 4 + js/ui/components/UploadModal.js | 80 +++--- js/ui/components/editModalUpload.js | 90 ++++++- js/ui/components/uploadModalStorageUnlock.js | 106 ++++++++ js/ui/initEditModal.js | 6 + js/ui/initUploadModal.js | 6 + .../upload-edit-storage-unlock-gate.test.mjs | 247 ++++++++++++++++++ 7 files changed, 496 insertions(+), 43 deletions(-) create mode 100644 js/ui/components/uploadModalStorageUnlock.js create mode 100644 tests/upload-edit-storage-unlock-gate.test.mjs diff --git a/js/ui/components/EditModal.js b/js/ui/components/EditModal.js index 51fdd842..c441a00a 100644 --- a/js/ui/components/EditModal.js +++ b/js/ui/components/EditModal.js @@ -25,6 +25,7 @@ export class EditModal { storageService = null, getCurrentPubkey = null, safeEncodeNpub = null, + ensureSigner = null, eventTarget, container, } = {}) { @@ -39,6 +40,9 @@ export class EditModal { this.storageService = storageService || null; this.getCurrentPubkey = typeof getCurrentPubkey === "function" ? getCurrentPubkey : null; + // Shared signer gate (#56): lets the "Replace file" pickers unlock storage + // inline (cached keep-unlocked key or one passphrase prompt). + this.ensureSigner = typeof ensureSigner === "function" ? ensureSigner : null; this.mediaUploader = new MediaUploader({ r2Service, s3Service, diff --git a/js/ui/components/UploadModal.js b/js/ui/components/UploadModal.js index 05787993..28d45373 100644 --- a/js/ui/components/UploadModal.js +++ b/js/ui/components/UploadModal.js @@ -27,6 +27,11 @@ import { } from "../../nostrClientFacade.js"; import { UI_FEEDBACK_DELAY_MS } from "../../constants.js"; import { showConfirm } from "../confirmDialog.js"; +import { + ensureStorageUnlockedForUpload, + resetThumbnailPicker, + promptStoredNsecUnlock, +} from "./uploadModalStorageUnlock.js"; const INFO_HASH_PATTERN = /^[a-f0-9]{40}$/; @@ -55,6 +60,7 @@ export class UploadModal { container, onRequestStorageSettings, onRequestUnlock, + ensureSigner, getHashtagSuggestions, } = {}) { this.authService = authService || null; @@ -96,6 +102,10 @@ export class UploadModal { // Opens the login modal's unlock-saved-key (passphrase) flow — used when a // persisted nsec session is locked after a reload. this.onRequestUnlock = typeof onRequestUnlock === "function" ? onRequestUnlock : null; + // Shared signer gate (app.ensureEncryptionCapableSigner): silently restores a + // kept-unlocked key or shows ONE passphrase prompt, so the file pickers can + // unlock storage inline instead of dead-ending (#56). + this.ensureSigner = typeof ensureSigner === "function" ? ensureSigner : null; this.root = null; this.isVisible = false; @@ -530,9 +540,13 @@ export class UploadModal { return; } if (!this.isStorageUnlocked) { - this.showError("Please unlock storage before selecting a file."); - e.target.value = ""; - return; + // #56: unlock inline (silent restore or one passphrase prompt) and + // continue with this same file pick instead of erroring. + const unlocked = await this.ensureStorageUnlockedForUpload(); + if (!unlocked) { + e.target.value = ""; + return; + } } // Start new session @@ -628,12 +642,19 @@ export class UploadModal { } async handleThumbnailSelection(file) { - if (!this.storageConfigured || !this.isStorageUnlocked) { - // We can't upload yet. Just hold it in state or warn? - // Since UI disables the browse button until unlocked (mostly), we assume safe. - // But if they just unlocked, we're good. + if (!this.storageConfigured) { + this.showError("Configure storage before uploading a thumbnail."); + this.resetThumbnailPicker(); return; } + if (!this.isStorageUnlocked) { + // #56: prompt to unlock inline instead of silently dropping the pick. + const unlocked = await this.ensureStorageUnlockedForUpload(); + if (!unlocked) { + this.resetThumbnailPicker(); + return; + } + } const currentUploadId = ++this.thumbnailUploadId; @@ -1011,41 +1032,18 @@ export class UploadModal { } } - // Returns true if a locked persisted-nsec session was detected (for the active - // account) and the re-unlock (passphrase) flow was opened, so the caller can stop. + // #36 / #56 storage-unlock glue lives in uploadModalStorageUnlock.js (file-size + // budget); these stay as instance methods so callers/tests are unchanged. + async ensureStorageUnlockedForUpload() { + return ensureStorageUnlockedForUpload(this); + } + + resetThumbnailPicker() { + resetThumbnailPicker(this); + } + promptStoredNsecUnlock(pubkey) { - const client = this.authService?.nostrClient; - if (!client || typeof client.getStoredSessionActorMetadata !== "function") { - return false; - } - let meta = null; - try { - meta = client.getStoredSessionActorMetadata(); - } catch (err) { - return false; - } - if (!meta || meta.hasEncryptedKey !== true || meta.source !== "nsec") { - return false; - } - const metaPubkey = - typeof meta.pubkey === "string" ? meta.pubkey.trim().toLowerCase() : ""; - const activePubkey = - typeof pubkey === "string" ? pubkey.trim().toLowerCase() : ""; - if (activePubkey && metaPubkey && metaPubkey !== activePubkey) { - return false; - } - if (typeof this.onRequestUnlock !== "function") { - return false; - } - this.showError( - "Your saved key is locked after reloading. Re-enter your passphrase to unlock it, then try the upload again.", - ); - try { - this.onRequestUnlock(); - } catch (err) { - userLogger.warn("[UploadModal] Failed to open saved-key unlock flow:", err); - } - return true; + return promptStoredNsecUnlock(this, pubkey); } updateLockUi() { diff --git a/js/ui/components/editModalUpload.js b/js/ui/components/editModalUpload.js index 5317e731..d75a3ca2 100644 --- a/js/ui/components/editModalUpload.js +++ b/js/ui/components/editModalUpload.js @@ -7,6 +7,10 @@ // and its rendered root (`context`). import { devLogger, userLogger } from "../../utils/logger.js"; +import { + getActiveSigner, + requestDefaultExtensionPermissions, +} from "../../nostrClientFacade.js"; // Set a field's value AND mark it edited/unlocked so EditModal.collect treats it // as a user change (isEditing() reads readOnly===false || dataset.isEditing). @@ -24,7 +28,8 @@ function setStatus(el, text) { // Resolve the active storage connection and surface a user-facing reason when // upload isn't possible. Returns the connection on success, null otherwise. -async function ensureUploadable(modal) { +// `deps` are injectable for tests only. +export async function ensureUploadable(modal, deps = {}) { if (!modal.mediaUploader) { modal.showError("Uploads are unavailable in this view."); return null; @@ -40,7 +45,88 @@ async function ensureUploadable(modal) { return null; } if (!conn.unlocked || !conn.credentials) { - modal.showError("Unlock your storage in the profile modal, then try again."); + // #56: unlock inline (kept-unlocked restore or one passphrase prompt) and + // continue with this same file pick instead of bouncing the user to the + // profile modal. + return tryInlineStorageUnlock(modal, deps); + } + return conn; +} + +const UNLOCK_FALLBACK_MESSAGE = + "Unlock your storage in the profile modal, then try again."; + +async function tryInlineStorageUnlock(modal, deps = {}) { + const getSigner = + typeof deps.getSigner === "function" ? deps.getSigner : getActiveSigner; + const requestPermissions = + typeof deps.requestPermissions === "function" + ? deps.requestPermissions + : requestDefaultExtensionPermissions; + + const pubkey = + typeof modal.getCurrentPubkey === "function" ? modal.getCurrentPubkey() : null; + if (!pubkey || !modal.storageService) { + modal.showError(UNLOCK_FALLBACK_MESSAGE); + return null; + } + + if (typeof modal.ensureSigner === "function") { + let gate = null; + try { + gate = await modal.ensureSigner({ + pubkey, + need: "encrypt", + promptMessage: + "Re-enter your PIN / passphrase to unlock storage for this upload.", + }); + } catch (error) { + devLogger.warn("[editModalUpload] Signer gate failed:", error); + } + if (gate && gate.ok !== true) { + // Cancel aborts quietly; a bad passphrase already showed its own toast. + if (gate.reason !== "cancelled" && gate.reason !== "bad-passphrase") { + modal.showError(UNLOCK_FALLBACK_MESSAGE); + } + return null; + } + } + + const signer = getSigner(); + if ( + !signer || + (typeof signer.nip44Decrypt !== "function" && + typeof signer.nip04Decrypt !== "function") + ) { + modal.showError(UNLOCK_FALLBACK_MESSAGE); + return null; + } + + try { + if (signer.type === "extension" || signer.type === "nip07") { + const permissionResult = await requestPermissions(); + if (!permissionResult?.ok) { + modal.showError("Extension permissions are required to unlock storage."); + return null; + } + } + await modal.storageService.unlock(pubkey, { signer }); + } catch (error) { + userLogger.error("[editModalUpload] Storage unlock failed:", error); + modal.showError( + "Failed to unlock storage: " + (error?.message || String(error)), + ); + return null; + } + + let conn = null; + try { + conn = await modal.mediaUploader.resolveActiveConnection(); + } catch (error) { + devLogger.warn("[editModalUpload] Failed to re-resolve storage connection:", error); + } + if (!conn?.unlocked || !conn?.credentials) { + modal.showError(UNLOCK_FALLBACK_MESSAGE); return null; } return conn; diff --git a/js/ui/components/uploadModalStorageUnlock.js b/js/ui/components/uploadModalStorageUnlock.js new file mode 100644 index 00000000..f5b4a1f7 --- /dev/null +++ b/js/ui/components/uploadModalStorageUnlock.js @@ -0,0 +1,106 @@ +// js/ui/components/uploadModalStorageUnlock.js +// +// Storage-unlock glue for the Upload modal (#36 / #56), kept out of +// UploadModal.js to stay within its file-size budget. Every function operates +// on the UploadModal instance (`modal`); UploadModal exposes them as thin +// prototype delegates so callers and tests keep using the instance methods. + +import { devLogger, userLogger } from "../../utils/logger.js"; + +// #56: the file pickers unlock storage inline instead of dead-ending. The +// shared signer gate (app.ensureEncryptionCapableSigner) restores a cached +// keep-unlocked key silently or shows ONE passphrase prompt; handleUnlock +// then unlocks storage with the restored signer (extension permissions + +// credential reload included). Returns true when the upload can proceed. +// Cancel / bad-passphrase abort quietly (the gate surfaces its own toast for +// a wrong passphrase); other gate failures fall through to handleUnlock so +// its existing messaging (saved-key flow / "no signer") still applies. +export async function ensureStorageUnlockedForUpload(modal) { + if (!modal.storageService) return false; + const pubkey = modal.getCurrentPubkey ? modal.getCurrentPubkey() : null; + if (!pubkey) return false; + + if (!modal.storageService.isUnlocked(pubkey)) { + if (typeof modal.ensureSigner === "function") { + let gate = null; + try { + gate = await modal.ensureSigner({ + pubkey, + need: "encrypt", + promptMessage: + "Re-enter your PIN / passphrase to unlock storage for this upload.", + }); + } catch (error) { + devLogger?.warn?.("[UploadModal] Signer gate failed:", error); + } + if ( + gate && + gate.ok !== true && + (gate.reason === "cancelled" || gate.reason === "bad-passphrase") + ) { + return false; + } + } + await modal.handleUnlock(); + } + + const unlocked = modal.storageService.isUnlocked(pubkey); + if (unlocked && (!modal.isStorageUnlocked || !modal.activeCredentials)) { + modal.isStorageUnlocked = true; + modal.updateLockUi(); + await modal.loadFromStorage(); + } + modal.isStorageUnlocked = unlocked; + return unlocked; +} + +// Undo the picker-side effects of a thumbnail selection that couldn't upload +// (placeholder shows the picked filename and the URL input is disabled). +export function resetThumbnailPicker(modal) { + if (modal.inputs?.thumbnailFile) { + modal.inputs.thumbnailFile.value = ""; + } + const thumbnail = modal.inputs?.thumbnail; + if (thumbnail) { + thumbnail.placeholder = "https://example.com/thumbnail.jpg"; + thumbnail.disabled = false; + } +} + +// Returns true if a locked persisted-nsec session was detected (for the active +// account) and the re-unlock (passphrase) flow was opened, so the caller can +// stop instead of dead-ending with "No signer available". +export function promptStoredNsecUnlock(modal, pubkey) { + const client = modal.authService?.nostrClient; + if (!client || typeof client.getStoredSessionActorMetadata !== "function") { + return false; + } + let meta = null; + try { + meta = client.getStoredSessionActorMetadata(); + } catch (err) { + return false; + } + if (!meta || meta.hasEncryptedKey !== true || meta.source !== "nsec") { + return false; + } + const metaPubkey = + typeof meta.pubkey === "string" ? meta.pubkey.trim().toLowerCase() : ""; + const activePubkey = + typeof pubkey === "string" ? pubkey.trim().toLowerCase() : ""; + if (activePubkey && metaPubkey && metaPubkey !== activePubkey) { + return false; + } + if (typeof modal.onRequestUnlock !== "function") { + return false; + } + modal.showError( + "Your saved key is locked after reloading. Re-enter your passphrase to unlock it, then try the upload again.", + ); + try { + modal.onRequestUnlock(); + } catch (err) { + userLogger.warn("[UploadModal] Failed to open saved-key unlock flow:", err); + } + return true; +} diff --git a/js/ui/initEditModal.js b/js/ui/initEditModal.js index 2f4cc9bb..78244c7d 100644 --- a/js/ui/initEditModal.js +++ b/js/ui/initEditModal.js @@ -51,6 +51,12 @@ export function initEditModal({ storageService, getCurrentPubkey, safeEncodeNpub, + // Shared signer gate (#56): lets the "Replace file" pickers unlock storage + // inline (cached keep-unlocked key or one passphrase prompt). + ensureSigner: (options) => + typeof app?.ensureEncryptionCapableSigner === "function" + ? app.ensureEncryptionCapableSigner(options) + : null, eventTarget: events, container, }); diff --git a/js/ui/initUploadModal.js b/js/ui/initUploadModal.js index 06ccc9e8..c70e9d6d 100644 --- a/js/ui/initUploadModal.js +++ b/js/ui/initUploadModal.js @@ -61,6 +61,12 @@ export function initUploadModal({ app.profileController.show("storage"); } }, + // Shared signer gate (#56): restores a kept-unlocked key silently or shows + // one passphrase prompt so the file pickers can unlock storage inline. + ensureSigner: (options) => + typeof app?.ensureEncryptionCapableSigner === "function" + ? app.ensureEncryptionCapableSigner(options) + : null, // Open the login modal's unlock-saved-key (passphrase) flow when a persisted // nsec session is locked after a reload. onRequestUnlock: () => { diff --git a/tests/upload-edit-storage-unlock-gate.test.mjs b/tests/upload-edit-storage-unlock-gate.test.mjs new file mode 100644 index 00000000..a90912e8 --- /dev/null +++ b/tests/upload-edit-storage-unlock-gate.test.mjs @@ -0,0 +1,247 @@ +// #56: when storage is locked, the upload modal's file pickers and the edit +// modal's "Replace file" pickers must unlock inline — silent kept-unlocked +// restore or ONE passphrase prompt via the shared signer gate — and continue +// with the same file pick, instead of dead-ending ("Please unlock storage…", +// a silent no-op, or "Unlock your storage in the profile modal"). +// +// test_integrity_note: +// change_type: ["new_tests"] +// scenarios: +// - id: SCN-upload-picker-inline-unlock +// given: "storage locked for the active pubkey; a signer gate that succeeds / is cancelled / fails" +// when: "ensureStorageUnlockedForUpload / handleThumbnailSelection run" +// then: "gate ok -> storage unlocked and pick proceeds; cancel/bad-passphrase -> quiet abort; other failures fall through to the regular unlock messaging" +// - id: SCN-edit-replace-file-inline-unlock +// given: "the edit modal's resolved storage connection is locked" +// when: "ensureUploadable runs" +// then: "the gate + storageService.unlock run and the re-resolved unlocked connection is returned; cancel aborts without unlocking" +// observable_outcomes: +// - "storageService.unlock called exactly when the gate allows it" +// - "cancelled gate -> no unlock attempt, no error toast" +// - "thumbnail picker state reset when the unlock is refused" +// determinism_controls: +// - "pure fake ctx objects; no DOM, no timers, no network" +// anti_cheat_rationale: +// prevents: ["hard-coded return value", "over-mocking internal logic"] +// relaxation: +// did_relax_any_assertion: false + +import test from "node:test"; +import { strict as assert } from "node:assert"; +import { UploadModal } from "../js/ui/components/UploadModal.js"; +import { ensureUploadable } from "../js/ui/components/editModalUpload.js"; + +const PUBKEY = "a".repeat(64); +const ensureUnlocked = UploadModal.prototype.ensureStorageUnlockedForUpload; +const handleThumb = UploadModal.prototype.handleThumbnailSelection; + +// --- UploadModal.ensureStorageUnlockedForUpload --- + +function makeUploadCtx({ unlockedAfterHandleUnlock = true, gate } = {}) { + const calls = { handleUnlock: 0, loadFromStorage: 0, errors: [], gates: [] }; + let unlocked = false; + const ctx = { + getCurrentPubkey: () => PUBKEY, + storageService: { isUnlocked: () => unlocked }, + ensureSigner: + gate === undefined + ? null + : async (options) => { + calls.gates.push(options); + return gate; + }, + handleUnlock: async () => { + calls.handleUnlock += 1; + if (unlockedAfterHandleUnlock) unlocked = true; + }, + updateLockUi: () => {}, + loadFromStorage: async () => { + calls.loadFromStorage += 1; + ctx.activeCredentials = { accessKeyId: "k" }; + }, + showError: (m) => calls.errors.push(m), + isStorageUnlocked: false, + activeCredentials: null, + }; + return { ctx, calls, setUnlocked: (v) => (unlocked = v) }; +} + +test("upload picker: gate ok -> unlocks via handleUnlock and proceeds", async () => { + const { ctx, calls } = makeUploadCtx({ gate: { ok: true } }); + const result = await ensureUnlocked.call(ctx); + assert.equal(result, true); + assert.equal(calls.gates.length, 1, "signer gate consulted once"); + assert.equal(calls.gates[0].need, "encrypt"); + assert.equal(calls.gates[0].pubkey, PUBKEY); + assert.equal(calls.handleUnlock, 1, "storage unlock ran after the gate"); + assert.equal(ctx.isStorageUnlocked, true); + assert.equal(calls.errors.length, 0); +}); + +test("upload picker: cancelled gate aborts quietly (no unlock, no toast)", async () => { + const { ctx, calls } = makeUploadCtx({ + gate: { ok: false, reason: "cancelled" }, + }); + assert.equal(await ensureUnlocked.call(ctx), false); + assert.equal(calls.handleUnlock, 0, "no unlock attempt after cancel"); + assert.equal(calls.errors.length, 0, "cancel is silent"); + assert.equal(ctx.isStorageUnlocked, false); +}); + +test("upload picker: bad passphrase aborts without a second toast", async () => { + const { ctx, calls } = makeUploadCtx({ + gate: { ok: false, reason: "bad-passphrase" }, + }); + assert.equal(await ensureUnlocked.call(ctx), false); + assert.equal(calls.handleUnlock, 0); + assert.equal(calls.errors.length, 0, "the gate already showed its own toast"); +}); + +test("upload picker: non-user gate failures fall through to handleUnlock's messaging", async () => { + const { ctx, calls } = makeUploadCtx({ + gate: { ok: false, reason: "no-stored-key" }, + unlockedAfterHandleUnlock: false, + }); + assert.equal(await ensureUnlocked.call(ctx), false); + assert.equal(calls.handleUnlock, 1, "regular unlock path still runs"); +}); + +test("upload picker: already-unlocked storage loads credentials without any gate", async () => { + const { ctx, calls, setUnlocked } = makeUploadCtx({ gate: { ok: true } }); + setUnlocked(true); + assert.equal(await ensureUnlocked.call(ctx), true); + assert.equal(calls.gates.length, 0, "no prompt when storage is already unlocked"); + assert.equal(calls.handleUnlock, 0); + assert.equal(calls.loadFromStorage, 1, "stale credentials refreshed"); + assert.ok(ctx.activeCredentials); +}); + +test("thumbnail pick while locked: refused unlock resets the picker, no upload", async () => { + const calls = { reset: 0, uploads: 0 }; + const ctx = { + storageConfigured: true, + isStorageUnlocked: false, + ensureStorageUnlockedForUpload: async () => false, + resetThumbnailPicker: () => { + calls.reset += 1; + }, + mediaUploader: { + uploadThumbnail: async () => { + calls.uploads += 1; + return { url: "https://x/thumb.jpg" }; + }, + }, + showError: () => {}, + }; + await handleThumb.call(ctx, { name: "t.jpg" }); + assert.equal(calls.reset, 1, "picker state restored"); + assert.equal(calls.uploads, 0, "upload never started"); +}); + +// --- editModalUpload.ensureUploadable (Edit modal "Replace file") --- + +function makeEditCtx({ gate, unlockThrows = false } = {}) { + const calls = { unlock: [], errors: [], gates: [], resolves: 0 }; + let unlocked = false; + const conn = () => ({ + configured: true, + unlocked, + credentials: unlocked ? { accessKeyId: "k" } : null, + provider: "s3", + }); + const modal = { + mediaUploader: { + resolveActiveConnection: async () => { + calls.resolves += 1; + return conn(); + }, + }, + storageService: { + unlock: async (pubkey, opts) => { + calls.unlock.push({ pubkey, signer: opts?.signer }); + if (unlockThrows) throw new Error("boom"); + unlocked = true; + }, + }, + getCurrentPubkey: () => PUBKEY, + ensureSigner: + gate === undefined + ? null + : async (options) => { + calls.gates.push(options); + return gate; + }, + showError: (m) => calls.errors.push(m), + }; + return { modal, calls }; +} + +const capableSigner = { nip44Decrypt: () => "x" }; + +test("edit replace-file: locked connection unlocks inline and returns the live connection", async () => { + const { modal, calls } = makeEditCtx({ gate: { ok: true } }); + const conn = await ensureUploadable(modal, { getSigner: () => capableSigner }); + assert.ok(conn, "connection returned"); + assert.equal(conn.unlocked, true); + assert.ok(conn.credentials, "credentials present after inline unlock"); + assert.equal(calls.gates.length, 1, "signer gate consulted"); + assert.deepEqual(calls.unlock[0].pubkey, PUBKEY); + assert.equal(calls.errors.length, 0); + assert.equal(calls.resolves, 2, "connection re-resolved after the unlock"); +}); + +test("edit replace-file: cancelled gate aborts without touching storage", async () => { + const { modal, calls } = makeEditCtx({ + gate: { ok: false, reason: "cancelled" }, + }); + const conn = await ensureUploadable(modal, { getSigner: () => capableSigner }); + assert.equal(conn, null); + assert.equal(calls.unlock.length, 0, "no unlock attempt after cancel"); + assert.equal(calls.errors.length, 0, "cancel is silent"); +}); + +test("edit replace-file: no gate + no capable signer keeps the actionable profile-modal message", async () => { + const { modal, calls } = makeEditCtx({}); + const conn = await ensureUploadable(modal, { getSigner: () => null }); + assert.equal(conn, null); + assert.equal(calls.unlock.length, 0); + assert.equal(calls.errors.length, 1); + assert.match(calls.errors[0], /profile modal/i); +}); + +test("edit replace-file: extension signer requests permissions before unlocking", async () => { + const { modal, calls } = makeEditCtx({ gate: { ok: true } }); + let permissionAsks = 0; + const conn = await ensureUploadable(modal, { + getSigner: () => ({ type: "extension", nip04Decrypt: () => "x" }), + requestPermissions: async () => { + permissionAsks += 1; + return { ok: true }; + }, + }); + assert.ok(conn); + assert.equal(permissionAsks, 1, "permissions requested for extension signers"); + assert.equal(calls.unlock.length, 1); +}); + +test("edit replace-file: unlock failure surfaces the error and returns null", async () => { + const { modal, calls } = makeEditCtx({ gate: { ok: true }, unlockThrows: true }); + const conn = await ensureUploadable(modal, { getSigner: () => capableSigner }); + assert.equal(conn, null); + assert.equal(calls.errors.length, 1); + assert.match(calls.errors[0], /Failed to unlock storage/); +}); + +test("edit replace-file: an already-unlocked connection passes straight through", async () => { + const { modal, calls } = makeEditCtx({ gate: { ok: true } }); + modal.mediaUploader.resolveActiveConnection = async () => ({ + configured: true, + unlocked: true, + credentials: { accessKeyId: "k" }, + provider: "s3", + }); + const conn = await ensureUploadable(modal, { getSigner: () => capableSigner }); + assert.ok(conn); + assert.equal(calls.gates.length, 0, "no prompt when already unlocked"); + assert.equal(calls.unlock.length, 0); +}); From 1934f9386a8845a03c7b465394f5679c0d09c891 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:01:31 -0400 Subject: [PATCH 02/27] publish: all-timeout content publishes are unconfirmed soft successes (#49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signAndPublishEvent (video publish/edit/repost/mirror), rebroadcastEvent, and revertVideo now use assertAnyRelayAcceptedOrUnconfirmed: when every relay times out without rejecting, the event was sent and almost always persisted, so it no longer surfaces as "Failed to share video". Explicit rejections still throw. Messaging stays honest at every consumer: - relay tally carries `unconfirmed` (now attached on edits too); describePublishOutcome maps 0-accepted+unconfirmed to a WARNING ("sent, no relay confirmed yet") — never a fake relay count, never a hard error - edit/revert controllers say "sent, not confirmed yet" when unconfirmed - deletes stay CONSERVATIVE: "deleted successfully" only when >=1 relay ACKed a tombstone or kind-5 (anyRelayAcceptedInSummaries); an unconfirmed revert no longer aborts deleteAllVersions midway, so the kind-5 always runs Tests: tests/relay-ack-unconfirmed-content.test.mjs (6). Co-Authored-By: Claude Fable 5 --- js/app.js | 19 ++- js/nostr/client.js | 19 ++- js/nostr/publishHelpers.js | 19 ++- js/nostrPublish.js | 43 ++++- js/ui/editModalController.js | 14 +- js/ui/revertModalController.js | 18 +- tests/relay-ack-unconfirmed-content.test.mjs | 168 +++++++++++++++++++ 7 files changed, 286 insertions(+), 14 deletions(-) create mode 100644 tests/relay-ack-unconfirmed-content.test.mjs diff --git a/js/app.js b/js/app.js index 592c0fcb..fe8aa063 100644 --- a/js/app.js +++ b/js/app.js @@ -92,6 +92,7 @@ import { assertAnyRelayAcceptedOrUnconfirmed, describePublishOutcome, readRelayPublishSummary, + anyRelayAcceptedInSummaries, } from "./nostrPublish.js"; import { getActiveSigner, @@ -5073,7 +5074,7 @@ class Application { this.showStatus("Deleting. Please wait.", { showSpinner: true }); try { - await this.nostrService.handleFullDeleteVideo({ + const deleteDetail = await this.nostrService.handleFullDeleteVideo({ videoRootId: rootId, video: targetVideo, pubkey: this.pubkey, @@ -5110,7 +5111,21 @@ class Application { } await this.loadVideos(); - this.showSuccess("All versions deleted successfully!"); + // #49 conservative delete semantics: only claim "deleted" when at least + // one relay ACKed something. An all-timeout run (tombstones + kind-5 sent + // but unconfirmed) must not read as a confirmed delete — the video may + // linger on relays until they catch up. + const deleteSummaries = [ + ...(deleteDetail?.result?.reverts || []), + ...(deleteDetail?.result?.deletes || []), + ].map((entry) => entry?.summary); + if (anyRelayAcceptedInSummaries(deleteSummaries)) { + this.showSuccess("All versions deleted successfully!"); + } else { + this.showSuccess( + "Delete request sent, but no relay has confirmed it yet — the video may reappear until relays catch up. Check again in a bit.", + ); + } if (storageLockedRemnant) { this.showStatus( "Heads up: the hosted file is still in your storage bucket because storage is locked. Unlock storage and delete again, or remove it from your bucket manually.", diff --git a/js/nostr/client.js b/js/nostr/client.js index 41f72c4b..e48f4b92 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -111,7 +111,7 @@ import "./maxListenerDiagnostics.js"; import { publishEventToRelay, publishEventToRelays, - assertAnyRelayAccepted, + assertAnyRelayAcceptedOrUnconfirmed, attachRelayPublishSummary, } from "../nostrPublish.js"; import { @@ -2960,13 +2960,17 @@ export class NostrClient { await this.ensureActiveSignerForPubkey(userPubkeyLower); try { - const { signedEvent } = await this.signAndPublishEvent(context.event, { + const { signedEvent, summary } = await this.signAndPublishEvent(context.event, { context: "edited video note", logName: "Edited video note", devLogLabel: "edited video note", resolveActiveSigner: (p) => this.signerManager.resolveActiveSigner(p), }); + // Same tally as publishVideo so the edit UI can tell an unconfirmed + // (all-timeout, #49) update apart from a confirmed one. + attachRelayPublishSummary(signedEvent, summary); + await handlePublishNip94(this, signedEvent, context); await handlePublishNip71(this, signedEvent, context); @@ -3216,7 +3220,10 @@ export class NostrClient { let publishSummary; try { - publishSummary = assertAnyRelayAccepted(publishResults, { + // #49: an all-timeout tombstone publish is an unconfirmed soft success + // (summary.unconfirmed=true) so a slow-ack revert/delete doesn't abort + // midway; explicit relay rejections still throw below. + publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { context: "video revert", }); } catch (publishError) { @@ -3237,6 +3244,12 @@ export class NostrClient { devLogger.log(`Revert event published to ${url}`) ); + if (publishSummary.unconfirmed) { + userLogger.warn( + "[nostr] Video revert was sent but no relay confirmed it within the timeout — proceeding as unconfirmed.", + ); + } + if (publishSummary.failed.length) { publishSummary.failed.forEach(({ url, error: relayError }) => { const reason = diff --git a/js/nostr/publishHelpers.js b/js/nostr/publishHelpers.js index 2b2e05f4..7c62b6e2 100644 --- a/js/nostr/publishHelpers.js +++ b/js/nostr/publishHelpers.js @@ -1,7 +1,7 @@ import { ENSURE_PRESENCE_REBROADCAST_COOLDOWN_SECONDS } from "../config.js"; import { publishEventToRelays, - assertAnyRelayAccepted, + assertAnyRelayAcceptedOrUnconfirmed, summarizePublishResults, } from "../nostrPublish.js"; import { @@ -469,7 +469,10 @@ export async function signAndPublishEvent({ let publishSummary; try { - publishSummary = assertAnyRelayAccepted(publishResults, { context }); + // #49: an all-timeout publish (sent, nothing rejected) is treated as an + // unconfirmed soft success — summary.unconfirmed=true — instead of a false + // "failed to publish". Explicit relay rejections still throw below. + publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { context }); } catch (publishError) { if (publishError?.relayFailures?.length) { const logLevel = rejectionLogLevel === "warn" ? "warn" : "error"; @@ -485,6 +488,12 @@ export async function signAndPublishEvent({ devLogger.log(`${logName} published to ${url}`); }); + if (publishSummary.unconfirmed) { + userLogger.warn( + `[nostr] ${logName} was sent but no relay confirmed it within the timeout — treating as an unconfirmed publish.`, + ); + } + if (publishSummary.failed.length) { publishSummary.failed.forEach(({ url, error: relayError }) => { const reason = @@ -1331,7 +1340,11 @@ export async function rebroadcastEvent({ client, eventId, options = {} }) { const publishResults = await publishEventToRelays(client?.pool, relays, rawEvent); try { - const summary = assertAnyRelayAccepted(publishResults, { context: "rebroadcast" }); + // #49: rebroadcast re-publishes an existing event, so an all-timeout is a + // safe unconfirmed success; only explicit rejections fall to the catch. + const summary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { + context: "rebroadcast", + }); return { ok: true, rebroadcast: true, diff --git a/js/nostrPublish.js b/js/nostrPublish.js index a9a7a751..c146f464 100644 --- a/js/nostrPublish.js +++ b/js/nostrPublish.js @@ -278,9 +278,16 @@ export function attachRelayPublishSummary(event, summary) { } const accepted = Array.isArray(summary?.accepted) ? summary.accepted.length : 0; const failed = Array.isArray(summary?.failed) ? summary.failed.length : 0; + // #49: carry the unconfirmed flag (all relays timed out, nothing rejected) + // so the outcome describer can say "sent, unconfirmed" instead of lying in + // either direction ("published to N relays" / "failed"). + const tally = + summary?.unconfirmed === true + ? { accepted, total: accepted + failed, unconfirmed: true } + : { accepted, total: accepted + failed }; try { Object.defineProperty(event, RELAY_PUBLISH_SUMMARY_KEY, { - value: { accepted, total: accepted + failed }, + value: tally, enumerable: false, configurable: true, writable: true, @@ -308,7 +315,11 @@ export function readRelayPublishSummary(event) { * @param {{ accepted?: number, total?: number }} tally * @returns {{ tone: "success"|"warning"|"error", message: string }} */ -export function describePublishOutcome({ accepted = 0, total = 0 } = {}) { +export function describePublishOutcome({ + accepted = 0, + total = 0, + unconfirmed = false, +} = {}) { const a = Number.isFinite(accepted) ? Math.max(0, Math.floor(accepted)) : 0; const t = Number.isFinite(total) ? Math.max(0, Math.floor(total)) : 0; @@ -317,6 +328,16 @@ export function describePublishOutcome({ accepted = 0, total = 0 } = {}) { return { tone: "success", message: "Video shared successfully!" }; } if (a <= 0) { + // #49: every relay timed out without rejecting — the event was sent and + // almost always persisted (it shows up after a refresh). Say exactly that: + // neither a hard failure nor a fake "published to N relays". + if (unconfirmed === true) { + return { + tone: "warning", + message: + "Video sent, but no relay has confirmed receiving it yet. It should appear shortly — refresh in a moment to double-check.", + }; + } return { tone: "error", message: @@ -335,6 +356,24 @@ export function describePublishOutcome({ accepted = 0, total = 0 } = {}) { }; } +// #49 delete semantics: a delete flow publishes several events (tombstone +// reverts + kind-5 chunks) and the UI must NOT claim "deleted" unless at least +// one relay actually ACKed something — a false "deleted" is worse than a false +// "failed". An empty list (nothing needed publishing) counts as confirmed. +export function anyRelayAcceptedInSummaries(summaries = []) { + const list = Array.isArray(summaries) ? summaries.filter(Boolean) : []; + if (!list.length) { + return true; + } + return list.some((summary) => { + const accepted = summary?.accepted; + if (Array.isArray(accepted)) { + return accepted.length > 0; + } + return Number.isFinite(accepted) && accepted > 0; + }); +} + // A RelayPublishError whose per-relay failures are ALL "publish timeout" means the // event was sent but not ACKed in the window — NOT that any relay rejected it. For // an idempotent, replaceable event this almost always persisted (users see it diff --git a/js/ui/editModalController.js b/js/ui/editModalController.js index 815891b2..2a74c06c 100644 --- a/js/ui/editModalController.js +++ b/js/ui/editModalController.js @@ -1,5 +1,6 @@ import { devLogger } from "../utils/logger.js"; import r2Service from "../services/r2Service.js"; +import { readRelayPublishSummary } from "../nostrPublish.js"; export default class EditModalController { constructor({ services, state, ui, callbacks, helpers }) { @@ -86,7 +87,7 @@ export default class EditModalController { } try { - await this.services.nostrService.handleEditVideoSubmit({ + const editedEvent = await this.services.nostrService.handleEditVideoSubmit({ originalEvent, updatedData, pubkey, @@ -139,7 +140,16 @@ export default class EditModalController { videosMap.clear(); } - this.ui.showSuccess("Video updated successfully!"); + // #49: an all-timeout publish reaches here flagged unconfirmed — say so + // honestly instead of claiming a confirmed update. + const tally = readRelayPublishSummary(editedEvent); + if (tally?.unconfirmed === true && !(tally.accepted > 0)) { + this.ui.showSuccess( + "Video update sent, but no relay has confirmed it yet — it may take a moment to propagate.", + ); + } else { + this.ui.showSuccess("Video updated successfully!"); + } if (editModal?.setSubmitState) { editModal.setSubmitState({ pending: false }); diff --git a/js/ui/revertModalController.js b/js/ui/revertModalController.js index 8815ff6d..deb76d68 100644 --- a/js/ui/revertModalController.js +++ b/js/ui/revertModalController.js @@ -91,8 +91,12 @@ export default class RevertModalController { this.revertModal.setBusy(true, "Reverting…"); try { + // #49: revertVideo no longer hard-fails when every relay merely times + // out (summary.unconfirmed) — track whether ANY relay actually ACKed so + // the success message stays honest. + let anyAccepted = false; for (const entry of entries) { - await this.services.nostrClient.revertVideo( + const revertResult = await this.services.nostrClient.revertVideo( { id: entry.id, pubkey: entry.pubkey, @@ -100,12 +104,22 @@ export default class RevertModalController { }, pubkey, ); + const accepted = revertResult?.summary?.accepted; + if (Array.isArray(accepted) && accepted.length > 0) { + anyAccepted = true; + } } await this.callbacks.loadVideos(); const timestampLabel = this.helpers.formatAbsoluteTimestamp(target.created_at); - this.ui.showSuccess(`Reverted to revision from ${timestampLabel}.`); + if (anyAccepted) { + this.ui.showSuccess(`Reverted to revision from ${timestampLabel}.`); + } else { + this.ui.showSuccess( + `Revert to the revision from ${timestampLabel} was sent, but no relay has confirmed it yet — check again in a moment.`, + ); + } this.revertModal.close(); this.callbacks.forceRefreshAllProfiles(); } catch (err) { diff --git a/tests/relay-ack-unconfirmed-content.test.mjs b/tests/relay-ack-unconfirmed-content.test.mjs new file mode 100644 index 00000000..3284e16d --- /dev/null +++ b/tests/relay-ack-unconfirmed-content.test.mjs @@ -0,0 +1,168 @@ +// #49: relay-ack false-negatives on the FLAGSHIP content flows. When no relay +// ACKs within the publish window but nothing rejected (all-timeout), the event +// was sent and almost always persisted — it must surface as an UNCONFIRMED soft +// success, not "Failed to share video". Equally important: the outcome messaging +// must stay honest — unconfirmed is never reported as "published to N relays", +// and a delete is only "deleted" when at least one relay ACKed something. +// +// test_integrity_note: +// change_type: ["new_tests"] +// scenarios: +// - id: SCN-content-publish-all-timeout +// given: "a relay pool where every relay times out (reason 'publish timeout') vs one that rejects" +// when: "signAndPublishEvent (video publish/edit path) runs" +// then: "all-timeout resolves with summary.unconfirmed=true; an explicit rejection still throws RelayPublishError" +// - id: SCN-honest-outcome-messaging +// given: "relay tallies including the new unconfirmed flag" +// when: "describePublishOutcome / attachRelayPublishSummary / anyRelayAcceptedInSummaries run" +// then: "unconfirmed -> warning ('sent, not confirmed'), never a relay-count success and never a hard error; deletes only 'confirmed' with >=1 ACK" +// observable_outcomes: +// - "signAndPublishEvent resolves (no throw) on all-timeout with unconfirmed summary" +// - "signAndPublishEvent still throws on explicit rejection" +// - "unconfirmed tally round-trips through attach/readRelayPublishSummary" +// - "describePublishOutcome(unconfirmed) is a warning with honest wording" +// determinism_controls: +// - "fake pool fires ok/failed handlers synchronously; no real sockets or timers race the 10s window" +// anti_cheat_rationale: +// prevents: ["hard-coded return value", "over-mocking internal logic", "snapshot rubber-stamping"] +// relaxation: +// did_relax_any_assertion: false + +import test from "node:test"; +import { strict as assert } from "node:assert"; +import { + RelayPublishError, + describePublishOutcome, + attachRelayPublishSummary, + readRelayPublishSummary, + anyRelayAcceptedInSummaries, +} from "../js/nostrPublish.js"; +import { signAndPublishEvent } from "../js/nostr/publishHelpers.js"; + +const PUB = "a".repeat(64); + +// Fake nostr-tools pool: publish([url]) returns a handle whose "failed" (or +// "ok") handler fires immediately with the scripted outcome for that relay. +function makePool(outcomes) { + let call = 0; + return { + publish(urls, _event) { + const outcome = outcomes[call % outcomes.length]; + call += 1; + return { + on(eventName, handler) { + if (outcome === "ok" && eventName === "ok") { + handler(); + } else if (outcome !== "ok" && eventName === "failed") { + handler(new Error(outcome)); + } + return this; + }, + }; + }, + }; +} + +const signer = { + pubkey: PUB, + signEvent: async (event) => ({ ...event, id: "signed-1", sig: "sig" }), +}; + +function publishWith(pool) { + return signAndPublishEvent({ + client: { + pubkey: PUB, + pool, + relays: ["wss://a.example", "wss://b.example"], + writeRelays: ["wss://a.example", "wss://b.example"], + }, + event: { kind: 30078, pubkey: PUB, created_at: 100, tags: [], content: "{}" }, + resolveActiveSigner: () => signer, + shouldRequestExtensionPermissions: () => false, + options: { context: "video note", logName: "Video note" }, + }); +} + +test("all-timeout video publish resolves as an unconfirmed soft success", async () => { + const result = await publishWith(makePool(["publish timeout", "publish timeout"])); + assert.equal(result.signedEvent.id, "signed-1"); + assert.equal(result.summary.unconfirmed, true, "flagged unconfirmed"); + assert.equal(result.summary.accepted.length, 0, "no fake acceptances"); + assert.equal(result.summary.failed.length, 2, "both timeouts recorded"); +}); + +test("an explicit relay rejection still hard-fails the publish", async () => { + await assert.rejects( + publishWith(makePool(["publish timeout", "blocked: spam"])), + RelayPublishError, + ); +}); + +test("a confirmed ack stays a confirmed (not unconfirmed) success", async () => { + const result = await publishWith(makePool(["ok", "publish timeout"])); + assert.equal(result.summary.unconfirmed, false); + assert.equal(result.summary.accepted.length, 1); +}); + +test("the attached tally carries the unconfirmed flag to the UI", () => { + const event = { id: "e1", kind: 30078 }; + attachRelayPublishSummary(event, { + accepted: [], + failed: [{ url: "wss://a" }, { url: "wss://b" }], + unconfirmed: true, + }); + assert.deepEqual(readRelayPublishSummary(event), { + accepted: 0, + total: 2, + unconfirmed: true, + }); + + // Confirmed publishes keep the exact legacy tally shape (no stray flag). + const confirmed = { id: "e2" }; + attachRelayPublishSummary(confirmed, { + accepted: [{ url: "wss://a" }], + failed: [], + }); + assert.deepEqual(readRelayPublishSummary(confirmed), { accepted: 1, total: 1 }); +}); + +test("describePublishOutcome: unconfirmed is an honest warning, not success or error", () => { + const out = describePublishOutcome({ accepted: 0, total: 3, unconfirmed: true }); + assert.equal(out.tone, "warning"); + assert.match(out.message, /no relay has confirmed/i); + assert.doesNotMatch(out.message, /\d+ relays?/, "must not fake a relay count"); + assert.doesNotMatch(out.message, /could not be published/i); + + // Without the flag, zero acceptance is still the hard error. + assert.equal(describePublishOutcome({ accepted: 0, total: 3 }).tone, "error"); + // A real acceptance is unaffected by a stray flag. + assert.equal( + describePublishOutcome({ accepted: 2, total: 3, unconfirmed: false }).tone, + "warning", + ); + assert.equal(describePublishOutcome({ accepted: 3, total: 3 }).tone, "success"); +}); + +test("delete confirmation: only >=1 relay ACK counts as a confirmed delete", () => { + const unconfirmedSummary = { accepted: [], failed: [{ url: "wss://a" }], unconfirmed: true }; + const confirmedSummary = { accepted: [{ url: "wss://b" }], failed: [] }; + + assert.equal( + anyRelayAcceptedInSummaries([unconfirmedSummary, unconfirmedSummary]), + false, + "all-timeout delete must NOT be reported as deleted", + ); + assert.equal( + anyRelayAcceptedInSummaries([unconfirmedSummary, confirmedSummary]), + true, + ); + assert.equal( + anyRelayAcceptedInSummaries([]), + true, + "nothing needed publishing counts as confirmed", + ); + assert.equal(anyRelayAcceptedInSummaries([null, undefined]), true); + // Numeric tally shape (from readRelayPublishSummary) also works. + assert.equal(anyRelayAcceptedInSummaries([{ accepted: 0, total: 2 }]), false); + assert.equal(anyRelayAcceptedInSummaries([{ accepted: 1, total: 2 }]), true); +}); From 99d1b1dd36b5db3b9e1800b973c95354f4e88782 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:01:45 -0400 Subject: [PATCH 03/27] tests: spec-correct four stale tests hidden by piped exit codes A per-file sweep (local suite runs were reading the PIPE's exit code, not npm's) surfaced four tests failing on HEAD: - hash-change-handler: logged-out default is deliberately TRENDING since 04621efc; test still expected most-recent-videos - session-actor / sessionActor / nsec-login: encoded the pre-fix "no-arg clearStoredSessionActor wipes EVERY account" behavior that 2026-07-02 deliberately removed (one account's logout must not destroy other saved accounts' keys). Replaced with stronger assertions: multi-account survival on no-arg clear, targeted forget, and the blocked-key security property asserted on the blocked account specifically. Integrity notes in each file; TODO #11b addendum documents the pipe lesson. Co-Authored-By: Claude Fable 5 --- tests/app/hash-change-handler.test.mjs | 29 +++++++- tests/nostr/session-actor.test.mjs | 22 ++++-- tests/nostr/sessionActor.test.mjs | 70 ++++++++++++++----- tests/nsec-login.test.mjs | 36 ++++++++-- todo/TODO_2026-06-20_pre-launch.md | 93 ++++++++++++++++++-------- 5 files changed, 190 insertions(+), 60 deletions(-) diff --git a/tests/app/hash-change-handler.test.mjs b/tests/app/hash-change-handler.test.mjs index 512153f3..d741d422 100644 --- a/tests/app/hash-change-handler.test.mjs +++ b/tests/app/hash-change-handler.test.mjs @@ -63,7 +63,30 @@ test("handleHashChange defaults to for-you when logged in", async () => { } }); -test("handleHashChange defaults to most-recent-videos when logged out", async () => { +// spec_correction: commit 04621efc (2026-06-24, "Trending defaults") made +// TRENDING the deliberate logged-out landing ("the best not-personalized +// landing", falling back to Recent only when FEATURE_TRENDING_FEED is off — +// see js/hashChangeHandler.js). This test still encoded the pre-change +// default (most-recent-videos) and has been failing since; the flag defaults +// to true, so the correct observable outcome is views/trending.html. +// +// test_integrity_note: +// change_type: ["spec_correction"] +// scenarios: +// - id: SCN-logged-out-default-view +// given: "no #view= hash and a logged-out user (FEATURE_TRENDING_FEED default-on)" +// when: "handleHashChange resolves the default view" +// then: "the Trending view loads and its init runs" +// observable_outcomes: +// - "loadView called with views/trending.html and the trending init fires" +// determinism_controls: +// - "JSDOM with fixed URL; injected logged-out application stub" +// anti_cheat_rationale: +// prevents: ["snapshot rubber-stamping"] +// relaxation: +// did_relax_any_assertion: false +// if_true_explain_spec_basis: "expectation moved to the newer intended default (equally strict)" +test("handleHashChange defaults to trending when logged out", async () => { const restore = setupDom("https://example.com"); try { @@ -77,7 +100,7 @@ test("handleHashChange defaults to most-recent-videos when logged out", async () loadViewCalls.push(viewUrl); }, viewInitRegistry: { - "most-recent-videos": async () => { + trending: async () => { initCalled = true; }, }, @@ -87,7 +110,7 @@ test("handleHashChange defaults to most-recent-videos when logged out", async () await handleHashChange(); - assert.deepEqual(loadViewCalls, ["views/most-recent-videos.html"]); + assert.deepEqual(loadViewCalls, ["views/trending.html"]); assert.equal(initCalled, true); } finally { restore(); diff --git a/tests/nostr/session-actor.test.mjs b/tests/nostr/session-actor.test.mjs index 0d9ec51b..7fc2a4ae 100644 --- a/tests/nostr/session-actor.test.mjs +++ b/tests/nostr/session-actor.test.mjs @@ -87,17 +87,31 @@ test("SessionActor", async (t) => { assert.equal(read.encryption.salt, actor.encryption.salt); }); - await t.test("clears stored session actor", async () => { + // spec_correction: no-arg clearStoredSessionActor() deliberately no longer + // wipes the per-account map (that wiped EVERY saved account's key on one + // account's logout — fixed 2026-07-02, same class as the NIP-46 map wipe). + // Forgetting an account is the targeted clearStoredSessionActor(pubkey). + // See tests/nostr/sessionActor.test.mjs for the full integrity note and the + // multi-account survival scenario. + await t.test("clears stored session actor (targeted; no-arg drops only the default slot)", async () => { const actor = { pubkey: "pubkey123", privateKeyEncrypted: "encrypteddata", encryption: { salt: "salt", iv: "iv" } }; sessionActor.persistSessionActor(actor); - sessionActor.clearStoredSessionActor(); - const read = sessionActor.readStoredSessionActorEntry(); - assert.equal(read, null); + // No-arg clear: default slot gone, but the account's key survives. + sessionActor.clearStoredSessionActor(); assert.equal(localStorage.getItem(sessionActor.SESSION_ACTOR_STORAGE_KEY), null); + assert.ok( + sessionActor.readStoredSessionActorEntry("pubkey123"), + "no-arg clear must not delete the account's stored key", + ); + + // Targeted clear actually forgets it. + sessionActor.clearStoredSessionActor("pubkey123"); + assert.equal(sessionActor.readStoredSessionActorEntry("pubkey123"), null); + assert.equal(sessionActor.readStoredSessionActorEntry(), null); }); }); diff --git a/tests/nostr/sessionActor.test.mjs b/tests/nostr/sessionActor.test.mjs index 143e558c..199e7a14 100644 --- a/tests/nostr/sessionActor.test.mjs +++ b/tests/nostr/sessionActor.test.mjs @@ -95,26 +95,60 @@ test("js/nostr/sessionActor.js", async (t) => { assert.strictEqual(decrypted, privateKey); }); - await t.test("Clear Stored Session Actor", async () => { - const pubkey = "0000000000000000000000000000000000000000000000000000000000000001"; - const actor = { - pubkey, - privateKeyEncrypted: "fake-encrypted-data", - encryption: { - salt: "fake-salt", - iv: "fake-iv", - iterations: 1000, - hash: "SHA-256", - algorithm: "AES-GCM", - version: 1 - }, - createdAt: Date.now(), + // spec_correction: no-arg clearStoredSessionActor() used to wipe the WHOLE + // per-account map — the data-loss bug where logging out of one account + // silently destroyed every other saved account's remembered nsec key. The + // deliberate fix (2026-07-02, same class as the NIP-46 session-map wipe) + // made no-arg clear ONLY the legacy v1 "last saved" slot; forgetting a + // specific account is clearStoredSessionActor(pubkey). This test encoded the + // dangerous pre-fix behavior; it now asserts the fixed semantics, including + // the multi-account survival property the fix exists for. + // + // test_integrity_note: + // change_type: ["spec_correction"] + // scenarios: + // - id: SCN-session-actor-targeted-clear + // given: "two accounts with persisted encrypted keys" + // when: "no-arg clear runs (logout-ish) and then a targeted clear" + // then: "no-arg leaves BOTH accounts retrievable by pubkey; targeted clear removes exactly one" + // observable_outcomes: + // - "clearStoredSessionActor() never deletes other accounts' keys" + // - "clearStoredSessionActor(pubkey) removes that account's entry" + // determinism_controls: + // - "in-memory localStorage; fixed pubkeys" + // anti_cheat_rationale: + // prevents: ["snapshot rubber-stamping"] + // relaxation: + // did_relax_any_assertion: false + // if_true_explain_spec_basis: "replaced with the fixed (stronger, multi-account) semantics" + await t.test("Clear semantics: targeted forget, no-arg never nukes other accounts", async () => { + const pubkeyA = "0000000000000000000000000000000000000000000000000000000000000001"; + const pubkeyB = "0000000000000000000000000000000000000000000000000000000000000002"; + const encryption = { + salt: "fake-salt", + iv: "fake-iv", + iterations: 1000, + hash: "SHA-256", + algorithm: "AES-GCM", + version: 1 }; + persistSessionActor({ pubkey: pubkeyA, privateKeyEncrypted: "cipher-a", encryption, createdAt: Date.now() }); + persistSessionActor({ pubkey: pubkeyB, privateKeyEncrypted: "cipher-b", encryption, createdAt: Date.now() }); - persistSessionActor(actor); - assert.ok(readStoredSessionActorEntry()); - + // No-arg clear (the logout-path call) drops only the "last saved" default… clearStoredSessionActor(); - assert.strictEqual(readStoredSessionActorEntry(), null); + assert.strictEqual(localStorage.getItem(SESSION_ACTOR_STORAGE_KEY), null); + // …while BOTH accounts' keys survive and stay switchable. + assert.ok(readStoredSessionActorEntry(pubkeyA), "account A must survive a no-arg clear"); + assert.ok(readStoredSessionActorEntry(pubkeyB), "account B must survive a no-arg clear"); + + // Targeted clear forgets exactly that account. + clearStoredSessionActor(pubkeyA); + assert.strictEqual(readStoredSessionActorEntry(pubkeyA), null, "targeted clear removes the account"); + assert.ok(readStoredSessionActorEntry(pubkeyB), "the other account is untouched"); + + clearStoredSessionActor(pubkeyB); + assert.strictEqual(readStoredSessionActorEntry(pubkeyB), null); + assert.strictEqual(readStoredSessionActorEntry(), null, "nothing left after every account is forgotten"); }); }); diff --git a/tests/nsec-login.test.mjs b/tests/nsec-login.test.mjs index a7bc34ca..32698989 100644 --- a/tests/nsec-login.test.mjs +++ b/tests/nsec-login.test.mjs @@ -12,9 +12,21 @@ let nostrClient; let getActiveSigner; let clearActiveSigner; let clearStoredSessionActor; +let listStoredSessionActorPubkeys; +let readStoredSessionActorEntry; let tools; let bytesToHex; +// No-arg clearStoredSessionActor() deliberately clears only the legacy +// "last saved" slot (per-account keys must survive another account's logout — +// fixed 2026-07-02). Tests that need a pristine store wipe every account. +function wipeStoredActors() { + for (const pk of listStoredSessionActorPubkeys?.() || []) { + clearStoredSessionActor(pk); + } + clearStoredSessionActor(); +} + before(async () => { tools = await ensureNostrTools(); const canonical = { ...tools }; @@ -24,7 +36,11 @@ before(async () => { ({ nostrClient } = await import("../js/nostrClientFacade.js")); ({ getActiveSigner, clearActiveSigner } = await import("../js/nostr/client.js")); - ({ clearStoredSessionActor } = await import("../js/nostr/sessionActor.js")); + ({ + clearStoredSessionActor, + listStoredSessionActorPubkeys, + readStoredSessionActorEntry, + } = await import("../js/nostr/sessionActor.js")); bytesToHex = (bytes) => { if (tools.bytesToHex) return tools.bytesToHex(bytes); @@ -35,7 +51,7 @@ before(async () => { after(() => { try { - clearStoredSessionActor?.(); + wipeStoredActors(); } catch (error) { // ignore } @@ -50,7 +66,7 @@ function makeKey() { describe("nsec login", () => { it("enforces the access-control validator before activating the signer", async () => { clearActiveSigner(); - clearStoredSessionActor(); + wipeStoredActors(); const { hex } = makeKey(); let seenPubkey = null; @@ -74,7 +90,7 @@ describe("nsec login", () => { it("persists an encrypted key and unlocks it with the passphrase", async () => { clearActiveSigner(); - clearStoredSessionActor(); + wipeStoredActors(); const { hex, pubkey } = makeKey(); const passphrase = "correct horse battery staple"; @@ -110,7 +126,7 @@ describe("nsec login", () => { it("unlock re-checks access control and forgets a blocked key", async () => { clearActiveSigner(); - clearStoredSessionActor(); + wipeStoredActors(); const { hex, pubkey } = makeKey(); const passphrase = "another secret passphrase"; @@ -132,10 +148,18 @@ describe("nsec login", () => { "a blocked key must not unlock", ); assert.equal(getActiveSigner(), null, "no signer activated for a blocked key"); + // The security property, asserted on the blocked account SPECIFICALLY (a + // no-arg metadata read can legitimately surface a different saved account + // via the single-entry fallback — that is not a leak of THIS key). + assert.equal( + readStoredSessionActorEntry(pubkey), + null, + "the blocked account's encrypted key is forgotten (cleared from storage)", + ); assert.equal( nostrClient.getStoredSessionActorMetadata(), null, - "a blocked key is forgotten (cleared from storage)", + "with no other saved accounts, no metadata survives the blocked-key cleanup", ); }); }); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index a94824a9..a27434f7 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -1166,6 +1166,13 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro trusted release gate. Audit e2e parallel-load flakiness too. ### 11b. SILENTLY-EXCLUDED unit tests — 20 files never run in CI (found 2026-06-23) +> **2026-07-03 addendum — stale-default failure was hiding in local runs:** +> `tests/app/hash-change-handler.test.mjs` had been failing since 04621efc +> (2026-06-24) made TRENDING the deliberate logged-out landing — the test still +> expected `most-recent-videos`. It went unnoticed locally because +> `npm run test:unit | tail` reports the PIPE's exit code, not npm's (lesson: +> never pipe the suite when reading its exit status). Spec-corrected to assert +> `views/trending.html` (integrity note in the test file); all 3 tests pass. `scripts/run-unit-tests.mjs` only collects `*.test.mjs|*.test.js` files whose **content contains `node:test`** (line ~54). 20 test files use bare `assert` + top-level await instead, so the runner skips them WITHOUT WARNING and they are @@ -1530,28 +1537,38 @@ that used it. - [ ] **VERIFY live on a phone**: modals should now be full-screen; tapping a video card shows the active lift/accent/marquee; profile modal tabs are the app-grid. -### 49. Relay-ack false-negatives on the generic content-publish path (HIGH-STAKES — deferred) -> No rush — only act if it starts causing problems. Same bug class as the list-publish -> fix (commit 47f2a9a8): when no relay ACKs within the 10s `RELAY_PUBLISH_TIMEOUT_MS` -> window, `assertAnyRelayAccepted` throws even though the event was sent and almost -> always persisted (works after refresh). Already fixed for the replaceable LISTS -> (subscriptions/blocks/relay-list/dm-hints/hashtags) via -> `assertAnyRelayAcceptedOrUnconfirmed`. -- [ ] **Generic content publish still hard-errors on all-timeout.** The - `signAndPublishEventHelper` (`js/nostr/publishHelpers.js:472`) + `rebroadcast` - (`:1334`) + `video revert` (`js/nostr/client.js:3064`) still use bare - `assertAnyRelayAccepted`, so a slow-ack **video publish/edit/delete/revert** - can show "Failed to share video" (or similar) when it actually published. - HIGH-STAKES because these are the flagship content flows. -- [ ] **Why it wasn't bundled in:** video publish feeds a relay summary into - `describePublishOutcome` (the "shared to N relays" / warning UI). Applying - `assertAnyRelayAcceptedOrUnconfirmed` here must keep that outcome honest (an - unconfirmed publish should read as "shared, couldn't confirm" — NOT a hard - failure and NOT a false "shared to N relays"). Needs its own careful pass + - tests around the video outcome-describer and delete semantics (a false - "deleted" is worse than a false "publish failed"). -- [ ] Deletions (kind 5) especially: decide whether an unconfirmed delete should - report success optimistically or stay conservative. +### 49. Relay-ack false-negatives on the generic content-publish path (HIGH-STAKES) +> Same bug class as the list-publish fix (commit 47f2a9a8): all-timeout ≠ rejection. +- [x] **DONE 2026-07-03 — generic content publish is unconfirmed-tolerant, with + HONEST outcome messaging at every consumer.** The three bare + `assertAnyRelayAccepted` call sites now use + `assertAnyRelayAcceptedOrUnconfirmed` (all-timeout → `summary.unconfirmed`, + soft success; explicit rejections STILL throw `RelayPublishError`): + - `signAndPublishEvent` (js/nostr/publishHelpers.js — video publish, edit, + repost, mirror, videoPublisher) + `rebroadcastEvent`. + - `revertVideo` (js/nostr/client.js) — so a slow-ack tombstone no longer + aborts `deleteAllVersions` midway (kind-5 hard delete now always runs). + Honesty plumbing (the reason this was deferred): + - The relay tally (`attachRelayPublishSummary`, now also attached on EDIT) + carries `unconfirmed`; `describePublishOutcome` maps accepted=0 + + unconfirmed → WARNING "sent, but no relay has confirmed receiving it + yet" — never a fake "published to N relays", never a hard failure. + Zero-accepted WITHOUT the flag stays the hard error. + - Edit: editModalController reads the tally and says "update sent, not + confirmed yet" instead of "updated successfully" when unconfirmed. + - Revert: revertModalController tracks per-entry summaries and only says + "Reverted…" when a relay ACKed; otherwise "sent, not confirmed yet". +- [x] **Delete semantics — decided CONSERVATIVE.** A false "deleted" is worse + than a false "failed": `anyRelayAcceptedInSummaries` (js/nostrPublish.js) + scans every tombstone + kind-5 summary; `handleDeleteModalConfirm` only + claims "All versions deleted successfully!" when ≥1 relay ACKed something, + else "Delete request sent, but no relay has confirmed it yet — the video + may reappear until relays catch up." (Empty set = nothing to publish = + confirmed.) Tests: `tests/relay-ack-unconfirmed-content.test.mjs` (6, incl. + end-to-end signAndPublishEvent all-timeout vs rejection). Lint + build green. +- [ ] **VERIFY on unstable:** with a slow/unreachable relay set, publish + edit + + delete a test video — messages should read "sent/unconfirmed", never + "failed", and the content should be correct after a refresh. ### 50. isDevMode — keep it; fix the test-harness gap (LOW priority) - Decision (2026-07): **KEEP `isDevMode`.** It's used in ~26 files, chiefly to gate @@ -1711,13 +1728,31 @@ passphrase-encrypted). Items 51, 56, 57 are all facets of that. merge-new already keeps grids fresh). ### 56. Upload & edit modals: prompt to unlock storage (PIN popup), don't just error (UX) -- [ ] When storage is locked, the **upload** and **edit** modals should open the - passphrase/PIN unlock popup instead of surfacing a raw error — for **both** the - video-file upload and the **thumbnail** upload. **#36** already added this - auto-open-login unlock for the **upload** modal; extend the same treatment to the - **edit** modal and the **thumbnail** upload path. -- [ ] Combine with **51**: once unlocked, cache the state so the user isn't re-prompted - on every upload/refresh. "Both would be nice" — actionable prompt AND cached unlock. +- [x] **DONE 2026-07-03.** All three locked-storage file-pick paths now unlock + INLINE and continue with the same pick, via the shared signer gate + (`app.ensureEncryptionCapableSigner`: silent kept-unlocked restore, else ONE + passphrase prompt with the keep-unlocked checkbox): + - **Upload modal, video file:** was `showError("Please unlock storage…")` + → now `ensureStorageUnlockedForUpload()` (new + `js/ui/components/uploadModalStorageUnlock.js`; UploadModal delegates — + also absorbed `promptStoredNsecUnlock` for the file-size budget). + - **Upload modal, thumbnail:** was a SILENT no-op → now the same inline + unlock; a refused unlock resets the picker (placeholder + URL input). + - **Edit modal, "Replace file" (video + thumbnail):** was "Unlock your + storage in the profile modal, then try again" → `ensureUploadable` + (js/ui/components/editModalUpload.js) now gates the signer, runs + `storageService.unlock`, and re-resolves the connection in-gesture + (extension signers still get the permission request first). + Cancel / bad-passphrase abort quietly (the gate toasts a wrong passphrase + itself); other failures fall through to the pre-existing messaging. + `ensureSigner` is threaded through initUploadModal / initEditModal from app. + Tests: `tests/upload-edit-storage-unlock-gate.test.mjs` (12). Lint + build green. +- [x] Combine with **51**: satisfied by the gate itself — it restores the cached + keep-unlocked key first, so an unlocked-once session never re-prompts. +- [ ] **VERIFY on unstable:** locked nsec session → pick a video file in the upload + modal (one PIN prompt, upload proceeds), pick a thumbnail (same), and use + Edit → Replace file (same). Cancel each prompt once — no error spam, picker + state restored. ### 57. nsec / NIP-46 accounts: encryption ops fail after refresh — re-prompt, don't blanket-error (BUG) - [ ] **Symptom:** adding/editing a hashtag on an **nsec**-logged-in account errors with From 096fe12c19c84a4e4d7cf43bd1a6bb06c0fa66b9 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:01:45 -0400 Subject: [PATCH 04/27] nostr: dev-gated REQ tracer at the pool.sub choke point (#9) The cold-login REQ storm's remaining emitters are unpinned (the suspected per-contact list loops don't exist; mutes are already batched). Every sub/list flows through the shimmed pool.sub, so recordReq() there aggregates kinds + relay fan-out + emitting call site per 10s window. Zero-cost when off; arm with localStorage bitvid:reqTrace=1 (auto in dev mode), read [req-trace] logs or window.__bitvidReqTrace.report(). One traced cold login on unstable will name the storm's sources; the batching/deferral pass follows from that data. Tests: tests/req-telemetry.test.mjs (3). Co-Authored-By: Claude Fable 5 --- js/nostr/reqTelemetry.js | 184 +++++++++++++++++++++++++++++ js/nostr/toolkit.js | 5 + tests/req-telemetry.test.mjs | 97 +++++++++++++++ todo/TODO_2026-06-20_pre-launch.md | 9 ++ 4 files changed, 295 insertions(+) create mode 100644 js/nostr/reqTelemetry.js create mode 100644 tests/req-telemetry.test.mjs diff --git a/js/nostr/reqTelemetry.js b/js/nostr/reqTelemetry.js new file mode 100644 index 00000000..16ad768f --- /dev/null +++ b/js/nostr/reqTelemetry.js @@ -0,0 +1,184 @@ +// js/nostr/reqTelemetry.js +// +// Dev-gated REQ tracer for TODO #9 (cold-login relay-REQ storm). Every +// subscription that goes through the shimmed pool.sub/pool.list choke point is +// recorded — kinds, relay fan-out, and the first call site outside the nostr +// plumbing — so one traced cold login tells us exactly WHICH code paths emit +// the burst (the storm's remaining sources are unpinned; see the TODO). +// +// Zero-cost when off: recordReq is a single boolean check. Enable with +// localStorage.setItem("bitvid:reqTrace", "1") (then reload) +// or automatically in dev mode. Read results from the periodic +// "[req-trace]" logs or on demand via window.__bitvidReqTrace.report(). + +import { isDevMode } from "../config.js"; +import { userLogger } from "../utils/logger.js"; + +const STORAGE_FLAG = "bitvid:reqTrace"; +const REPORT_WINDOW_MS = 10000; +const PLUMBING_FRAME = /reqTelemetry\.js|toolkit\.js|relaySubscriptionService|subscriptionManager|nostrPublish\.js|node_modules|^Error/i; + +function readStorageFlag() { + try { + return ( + typeof localStorage !== "undefined" && + localStorage.getItem(STORAGE_FLAG) === "1" + ); + } catch (error) { + return false; + } +} + +// First stack frame that is application code rather than pool plumbing. +export function extractCallSite(stack) { + if (typeof stack !== "string" || !stack) { + return "unknown"; + } + for (const rawLine of stack.split("\n")) { + const line = rawLine.trim(); + if (!line || PLUMBING_FRAME.test(line)) { + continue; + } + // "at fn (url:line:col)" / "fn@url:line:col" → keep it short but unique + // enough to identify the module (strip origin, keep path:line). + const match = line.match(/((?:[\w./-]+\/)?[\w.-]+\.m?js):(\d+)(?::\d+)?/); + if (match) { + const fnMatch = line.match(/at ([\w.<>$[\]]+) |^([\w.<>$[\]]+)@/); + const fn = fnMatch ? fnMatch[1] || fnMatch[2] : ""; + return `${fn ? `${fn} ` : ""}${match[1]}:${match[2]}`.trim(); + } + return line.slice(0, 120); + } + return "unknown"; +} + +export function createReqTelemetry({ + now = () => Date.now(), + log = (payload) => userLogger.info("[req-trace]", payload), + windowMs = REPORT_WINDOW_MS, +} = {}) { + const state = { + enabled: false, + entries: new Map(), // key: `${kinds}|${callSite}` → { kinds, callSite, count, relayFanout } + total: 0, + relayFanout: 0, + windowStart: 0, + timer: null, + }; + + const reset = () => { + state.entries.clear(); + state.total = 0; + state.relayFanout = 0; + state.windowStart = now(); + }; + + const record = (relayCount, filters, stack) => { + if (!state.enabled) { + return; + } + const list = Array.isArray(filters) ? filters : filters ? [filters] : []; + const kinds = Array.from( + new Set( + list.flatMap((filter) => + Array.isArray(filter?.kinds) ? filter.kinds : [], + ), + ), + ) + .sort((a, b) => a - b) + .join(","); + const callSite = extractCallSite(stack); + const key = `${kinds || "?"}|${callSite}`; + const entry = state.entries.get(key) || { + kinds: kinds || "?", + callSite, + count: 0, + relayFanout: 0, + }; + entry.count += 1; + entry.relayFanout += Number.isFinite(relayCount) ? relayCount : 0; + state.entries.set(key, entry); + state.total += 1; + state.relayFanout += Number.isFinite(relayCount) ? relayCount : 0; + }; + + const report = ({ emit = true } = {}) => { + const elapsedMs = Math.max(1, now() - state.windowStart); + const rows = Array.from(state.entries.values()).sort( + (a, b) => b.count - a.count, + ); + const payload = { + windowSeconds: Math.round(elapsedMs / 100) / 10, + totalSubs: state.total, + totalRelayReqs: state.relayFanout, + perSecond: Math.round((state.relayFanout / elapsedMs) * 10000) / 10, + byKindsAndCaller: rows.slice(0, 15).map((row) => ({ + kinds: row.kinds, + caller: row.callSite, + subs: row.count, + relayReqs: row.relayFanout, + })), + }; + if (emit && state.total > 0) { + log(payload); + } + return payload; + }; + + const start = () => { + if (state.enabled) { + return; + } + state.enabled = true; + reset(); + if (typeof setInterval === "function" && !state.timer) { + state.timer = setInterval(() => { + report(); + reset(); + }, windowMs); + if (typeof state.timer?.unref === "function") { + state.timer.unref(); + } + } + }; + + const stop = () => { + state.enabled = false; + if (state.timer) { + clearInterval(state.timer); + state.timer = null; + } + }; + + return { + get enabled() { + return state.enabled; + }, + start, + stop, + reset, + record, + report, + }; +} + +const defaultTelemetry = createReqTelemetry(); + +// Auto-arm in dev mode or when the localStorage flag is set; expose a console +// handle either way so it can be started on a production build too. +if (isDevMode || readStorageFlag()) { + defaultTelemetry.start(); +} +if (typeof window !== "undefined") { + window.__bitvidReqTrace = defaultTelemetry; +} + +// Hot-path hook for the pool shim: one boolean check when tracing is off. +export function recordReq(relayCount, filters) { + if (!defaultTelemetry.enabled) { + return; + } + defaultTelemetry.record(relayCount, filters, new Error().stack); +} + +export default defaultTelemetry; diff --git a/js/nostr/toolkit.js b/js/nostr/toolkit.js index 05d47e0b..b7e21855 100644 --- a/js/nostr/toolkit.js +++ b/js/nostr/toolkit.js @@ -3,6 +3,7 @@ import { isDevMode } from "../config.js"; import { nostrToolsReady } from "../nostrToolsBootstrap.js"; import { devLogger, userLogger } from "../utils/logger.js"; +import { recordReq } from "./reqTelemetry.js"; /** * The default relay set bitvid bootstraps with before loading a user's @@ -524,6 +525,10 @@ export function shimLegacySimplePoolMethods(pool) { const { relayList, filterList, requests } = createLegacySubscriptionContext(relays, filters); + // #9 REQ tracer: every sub/list passes through here — record kinds, + // fan-out, and the emitting call site (no-op unless tracing is armed). + recordReq(relayList.length, filterList); + const listeners = { event: new Set(), eose: new Set(), diff --git a/tests/req-telemetry.test.mjs b/tests/req-telemetry.test.mjs new file mode 100644 index 00000000..c98e1303 --- /dev/null +++ b/tests/req-telemetry.test.mjs @@ -0,0 +1,97 @@ +// #9: the cold-login REQ storm's remaining emitters are unpinned, so a +// dev-gated tracer at the pool.sub choke point aggregates every subscription +// by kinds + emitting call site. These tests cover the aggregation, the +// call-site extraction (must skip pool plumbing frames), and that the hot +// path is a no-op while tracing is off. +// +// test_integrity_note: +// change_type: ["new_tests"] +// scenarios: +// - id: SCN-req-trace-aggregation +// given: "a telemetry instance with an injected clock and log sink" +// when: "subs with various kinds/relay counts are recorded and report() runs" +// then: "rows are grouped by kinds+caller, sorted by count, with correct totals and REQ/s" +// observable_outcomes: +// - "disabled instance records nothing" +// - "report() totals equal the recorded subs and relay fan-out" +// - "plumbing frames (toolkit/relaySubscriptionService) never become the caller" +// determinism_controls: +// - "injected now(); no interval timers exercised; synthetic stacks" +// anti_cheat_rationale: +// prevents: ["hard-coded return value", "over-mocking internal logic"] +// relaxation: +// did_relax_any_assertion: false + +import "./test-helpers/setup-localstorage.mjs"; +import test from "node:test"; +import { strict as assert } from "node:assert"; +import { + createReqTelemetry, + extractCallSite, +} from "../js/nostr/reqTelemetry.js"; + +const STACK = (site) => + [ + "Error", + " at recordReq (http://x/js/nostr/reqTelemetry.js:180:5)", + " at legacySub (http://x/js/nostr/toolkit.js:529:7)", + ` at ${site}`, + ].join("\n"); + +test("records aggregate by kinds + caller and report totals are honest", () => { + let t = 0; + const logs = []; + const telem = createReqTelemetry({ + now: () => t, + log: (payload) => logs.push(payload), + }); + telem.start(); + t = 0; + telem.reset(); + + for (let i = 0; i < 3; i += 1) { + telem.record(8, [{ kinds: [30000], authors: ["a"] }], STACK("loadLists (http://x/js/subscriptions.js:757:10)")); + } + telem.record(4, [{ kinds: [0] }, { kinds: [10050] }], STACK("dmBoot (http://x/js/services/dmNostrService.js:127:3)")); + + t = 5000; // 5s window + const payload = telem.report({ emit: true }); + + assert.equal(payload.totalSubs, 4); + assert.equal(payload.totalRelayReqs, 28, "3×8 + 4 relay-level REQs"); + assert.equal(payload.perSecond, 5.6, "28 REQs over 5s"); + assert.equal(payload.byKindsAndCaller[0].kinds, "30000"); + assert.equal(payload.byKindsAndCaller[0].subs, 3); + assert.match(payload.byKindsAndCaller[0].caller, /subscriptions\.js:757/); + assert.equal(payload.byKindsAndCaller[1].kinds, "0,10050"); + assert.match(payload.byKindsAndCaller[1].caller, /dmNostrService\.js:127/); + assert.equal(logs.length, 1, "report emitted through the injected sink"); +}); + +test("disabled tracer records nothing; stop() disarms a started one", () => { + const telem = createReqTelemetry({ now: () => 0, log: () => {} }); + telem.record(8, [{ kinds: [30000] }], STACK("x (http://x/js/a.js:1:1)")); + assert.equal(telem.report({ emit: false }).totalSubs, 0, "off by default"); + + telem.start(); + telem.record(2, [{ kinds: [7] }], STACK("x (http://x/js/a.js:1:1)")); + telem.stop(); + telem.record(2, [{ kinds: [7] }], STACK("x (http://x/js/a.js:1:1)")); + assert.equal(telem.report({ emit: false }).totalSubs, 1, "no records after stop"); +}); + +test("call-site extraction skips pool plumbing and keeps module:line", () => { + assert.match( + extractCallSite(STACK("refresh (http://x/js/services/moderationService.js:843:20)")), + /moderationService\.js:843/, + ); + // Plumbing-only stack falls back to unknown rather than blaming the shim. + assert.equal( + extractCallSite( + "Error\n at legacySub (http://x/js/nostr/toolkit.js:529:7)\n at ensureSubscription (http://x/js/services/relaySubscriptionService.js:158:1)", + ), + "unknown", + ); + assert.equal(extractCallSite(""), "unknown"); + assert.equal(extractCallSite(undefined), "unknown"); +}); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index a27434f7..91dfedbb 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -712,6 +712,15 @@ toward freshness and looked identical. Gave each a structural identity: ones (DM relays, interest sets) until after first render. Multi-subsystem + sensitive (trust) → do as a dedicated perf pass, not a rushed change. Source still to pin: the loop issuing 30000/30002/30005/30015 per contact. +- [x] **REQ tracer SHIPPED (2026-07-03)** to pin those emitters empirically — + static search found NO per-contact list loops (mutes already batched), so + the hypothesis needs data. `js/nostr/reqTelemetry.js` hooks the shimmed + `pool.sub` choke point (every sub/list flows through it) and aggregates + kinds + relay fan-out + emitting call site per 10s window. Zero-cost off. + **To trace:** on unstable run `localStorage.setItem("bitvid:reqTrace","1")`, + reload (cold login), read the `[req-trace]` console logs (or + `window.__bitvidReqTrace.report()`); the top rows name the emitters and + the batching/deferral pass follows directly from that data. ### 22. Anti-spam: validate npub on application / submission forms - [x] **DONE 2026-06-24.** Added a shared, testable NIP-19 validator From 27be0301c5140f709bf6aef1fd9fa8f43a3f0487 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:04:29 -0400 Subject: [PATCH 05/27] =?UTF-8?q?todo:=20#10=20bounded=20investigation=20?= =?UTF-8?q?=E2=80=94=20null.fill=20is=20a=20streamx/fast-fifo=20use-after-?= =?UTF-8?q?destroy;=20fix=20is=20a=20vendored=203.0.16=20update=20as=20its?= =?UTF-8?q?=20own=20soaked=20task?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- todo/TODO_2026-06-20_pre-launch.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index 91dfedbb..62682d9d 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -1167,6 +1167,22 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro ### 10. WebTorrent `null.fill` seeding crash - [ ] Investigate the vendored `webtorrent.min.js` `null.fill` error on wire handshake; may require a bundle update or patch. +- [x] **Bounded investigation DONE 2026-07-03.** The vendored bundle is + webtorrent **2.5.11** (~2 years old; upstream latest is **3.0.16**). The + bundle has exactly two `.fill(` sites: a piece-bitfield fill and + `this.buffer.fill(void 0)` — fast-fifo's `clear()`, the queue streamx + uses under bittorrent-protocol's wire Duplex. "null.fill on wire + handshake" = that queue being touched after stream teardown (use-after- + destroy in the streamx/fast-fifo layer). No matching public upstream + issue found, but webtorrent 3.x now PINS `streamx` to an exact version + (2.25.0) — consistent with upstream having chased this bug class. +- [ ] **Fix path: vendored-bundle update to 3.0.16 as its OWN task** (playback- + critical → own branch/soak, NOT bundled with other work): rebuild + `js/webtorrent.min.js` + `.map`, re-test URL+magnet playback, seeding, + the beacon app (`torrent/`), and the SW streaming path; rollback = keep + the old bundle file. Check 2.x→3.x breaking changes against + `js/webtorrent.js` usage first. Without a repro for the crash, a blind + local patch is guesswork — the update is the honest fix. ### 11. Harden flaky tests (CI gate reliability) - [ ] `tests/ui/uploadModal-reset.test.mjs` ("UploadModal Reset Logic") intermittently From d1a578f3a2d0d8ac12f82c706677e34e870c511f Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:24:13 -0400 Subject: [PATCH 06/27] profile modal: always-visible "Acting as" identity card (#43) Identity pill top-center on every pane, straddling the header divider, so users always see which profile they're acting as (e.g. before uploading). Populated in renderSavedProfiles (same resolveMeta path as the Account pane card, so it refreshes on profile switch), hidden when logged out, and click-through jumps to the Account pane. Co-Authored-By: Claude Fable 5 --- components/profile-modal.html | 29 ++++++++++ css/tailwind.source.css | 73 +++++++++++++++++++++++++ js/ui/profileModalController.js | 31 +++++++++++ tests/profile-modal-controller.test.mjs | 53 ++++++++++++++++++ 4 files changed, 186 insertions(+) diff --git a/components/profile-modal.html b/components/profile-modal.html index 39d716a6..a4ee0f30 100644 --- a/components/profile-modal.html +++ b/components/profile-modal.html @@ -62,6 +62,35 @@ + + +
{ + this.selectPane("account"); + }); + } + if (this.permissionPromptCtaButton instanceof HTMLElement) { this.permissionPromptCtaButton.addEventListener("click", () => { if (this.permissionPromptCtaButton?.dataset?.action === "retry-auth-sync") { @@ -2564,6 +2577,24 @@ export class ProfileModalController { : this.globalProfileAvatar.alt || "Profile avatar"; } + // #43: keep the always-visible identity card in sync (hidden when logged + // out so the modal doesn't claim an identity that isn't active). + if (this.identityCard instanceof HTMLElement) { + this.identityCard.classList.toggle("hidden", !hasActiveProfile); + this.identityCard.setAttribute( + "aria-hidden", + hasActiveProfile ? "false" : "true", + ); + } + if (this.identityCardName instanceof HTMLElement) { + this.identityCardName.textContent = activeDisplayName; + } + if (this.identityCardAvatar instanceof HTMLImageElement) { + if (this.identityCardAvatar.src !== activeAvatarSrc) { + this.identityCardAvatar.src = activeAvatarSrc; + } + } + const listEl = this.switcherList; if (listEl instanceof HTMLElement) { listEl.textContent = ""; diff --git a/tests/profile-modal-controller.test.mjs b/tests/profile-modal-controller.test.mjs index 49f0aa24..74d773e0 100644 --- a/tests/profile-modal-controller.test.mjs +++ b/tests/profile-modal-controller.test.mjs @@ -726,6 +726,59 @@ for (const _ of [0]) { assert.equal(switcherNpubEl.textContent, expectedSwitcher); }); + // #43: the always-visible identity card mirrors the active profile on every + // pane (populated by renderSavedProfiles, so it also refreshes on account + // switch), hides when logged out, and click-through opens the Account pane. + test('identity card shows the acting profile, hides logged out, opens Account', async () => { + const sampleProfiles = [ + { + pubkey: defaultActorHex, + npub: 'npub1abcdefghijkmnopqrstuvwxyz1234567890example', + name: 'Primary Account', + picture: '', + providerId: 'nip07', + authType: 'nip07', + }, + ]; + + const controller = createController({ + services: { + formatShortNpub, + }, + }); + + await controller.load(); + + controller.state.setSavedProfiles(sampleProfiles); + controller.state.setActivePubkey(defaultActorHex); + controller.renderSavedProfiles(); + + assert.ok(controller.identityCard, 'identity card element is cached'); + assert.equal( + controller.identityCard.classList.contains('hidden'), + false, + 'visible while a profile is active', + ); + assert.equal(controller.identityCard.getAttribute('aria-hidden'), 'false'); + assert.equal(controller.identityCardName.textContent, 'Primary Account'); + + controller.identityCard.click(); + assert.equal( + controller.getActivePane(), + 'account', + 'click-through jumps to the Account pane', + ); + + controller.state.setActivePubkey(null); + controller.renderSavedProfiles(); + assert.equal( + controller.identityCard.classList.contains('hidden'), + true, + 'hidden when logged out (no claimed identity)', + ); + assert.equal(controller.identityCard.getAttribute('aria-hidden'), 'true'); + }); + test('renderSavedProfiles applies provider metadata', async () => { const sampleProfiles = [ { From 35a50611fe576e584cb2fcf93f0177d829949140 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:24:13 -0400 Subject: [PATCH 07/27] upload: in-modal "Upload destination" storage picker (#44) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The storage summary gains a connection +
Provider diff --git a/js/ui/components/UploadModal.js b/js/ui/components/UploadModal.js index 28d45373..9a1714ed 100644 --- a/js/ui/components/UploadModal.js +++ b/js/ui/components/UploadModal.js @@ -31,6 +31,8 @@ import { ensureStorageUnlockedForUpload, resetThumbnailPicker, promptStoredNsecUnlock, + pickTargetConnection, + renderConnectionPicker, } from "./uploadModalStorageUnlock.js"; const INFO_HASH_PATTERN = /^[a-f0-9]{40}$/; @@ -116,6 +118,8 @@ export class UploadModal { this.activeProvider = null; this.isStorageUnlocked = false; this.storageConfigured = false; + // #44: per-modal upload-destination override (null = account default). + this.selectedConnectionId = null; // Upload State this.videoUploadState = { @@ -247,6 +251,7 @@ export class UploadModal { progress: $("#upload-progress-container"), thumbnailProgress: $("#thumbnail-progress-container"), results: $("#upload-results-container"), + connectionPicker: $("#storage-connection-picker"), }; this.storageViews = { @@ -274,6 +279,9 @@ export class UploadModal { // Progress progress: $("#input-progress"), thumbnailProgress: $("#thumbnail-progress"), + + // #44: upload-destination picker + storageConnection: $("#select-storage-connection"), }; // Results (Generated Links) @@ -334,6 +342,14 @@ export class UploadModal { this.toggles.storageUnlock.addEventListener("click", () => this.handleUnlock()); } + if (this.inputs.storageConnection) { + // #44: switching the destination re-resolves summary + credentials. + this.inputs.storageConnection.addEventListener("change", () => { + this.selectedConnectionId = this.inputs.storageConnection.value || null; + void this.loadFromStorage(); + }); + } + // Manage Storage if (this.toggles.manageStorage) { this.toggles.manageStorage.addEventListener("click", () => { @@ -910,9 +926,9 @@ export class UploadModal { try { const connections = await this.storageService.listConnections(pubkey); - // Prefer default connection - const defaultConn = connections.find(c => c.meta?.defaultForUploads); - const targetConn = defaultConn || connections[0]; + // #44: a per-modal selection wins over the account default. + const { targetConn, defaultConn } = pickTargetConnection(this, connections); + renderConnectionPicker(this, connections, targetConn, defaultConn); if (targetConn) { this.storageConfigured = true; diff --git a/js/ui/components/uploadModalStorageUnlock.js b/js/ui/components/uploadModalStorageUnlock.js index f5b4a1f7..8915fb5b 100644 --- a/js/ui/components/uploadModalStorageUnlock.js +++ b/js/ui/components/uploadModalStorageUnlock.js @@ -1,9 +1,10 @@ // js/ui/components/uploadModalStorageUnlock.js // -// Storage-unlock glue for the Upload modal (#36 / #56), kept out of -// UploadModal.js to stay within its file-size budget. Every function operates -// on the UploadModal instance (`modal`); UploadModal exposes them as thin -// prototype delegates so callers and tests keep using the instance methods. +// Storage glue for the Upload modal (#36 / #56 unlock + #44 connection +// picker), kept out of UploadModal.js to stay within its file-size budget. +// Every function operates on the UploadModal instance (`modal`); UploadModal +// exposes them as thin prototype delegates so callers and tests keep using +// the instance methods. import { devLogger, userLogger } from "../../utils/logger.js"; @@ -67,6 +68,52 @@ export function resetThumbnailPicker(modal) { } } +// #44: resolve which configured connection this upload targets. A per-modal +// selection wins; a stale selection (connection deleted meanwhile) is reset; +// otherwise the account default, then the first configured connection. +export function pickTargetConnection(modal, connections) { + const list = Array.isArray(connections) ? connections : []; + const defaultConn = list.find((conn) => conn?.meta?.defaultForUploads) || null; + if (modal.selectedConnectionId) { + const selected = list.find((conn) => conn?.id === modal.selectedConnectionId); + if (selected) { + return { targetConn: selected, defaultConn }; + } + modal.selectedConnectionId = null; + } + return { targetConn: defaultConn || list[0] || null, defaultConn }; +} + +// #44: render the "Upload destination"
', + ); + const doc = dom.window.document; + global.HTMLElement = dom.window.HTMLElement; + return { + modal: { + selectedConnectionId: null, + inputs: { storageConnection: doc.getElementById("sel") }, + sourceSections: { connectionPicker: doc.getElementById("wrap") }, + getProviderLabel(provider) { + return provider === "cloudflare_r2" ? "Cloudflare R2" : provider; + }, + }, + doc, + }; +} + +test("picker renders options (default labelled, target selected); hidden under two connections", () => { + const { modal } = makeModal(); + + renderConnectionPicker(modal, [r2, b2, s3], b2, b2); + const wrap = modal.sourceSections.connectionPicker; + const options = Array.from(modal.inputs.storageConnection.options); + assert.equal(wrap.classList.contains("hidden"), false, "picker shown"); + assert.deepEqual( + options.map((o) => o.value), + ["c1", "c2", "c3"], + ); + assert.match(options[0].textContent, /Cloudflare R2 — vids/); + assert.match(options[1].textContent, /\(default\)$/, "default is labelled"); + assert.equal( + modal.inputs.storageConnection.value, + "c2", + "target connection pre-selected", + ); + + renderConnectionPicker(modal, [r2, b2, s3], s3, b2); + assert.equal(modal.inputs.storageConnection.value, "c3", "re-render follows the target"); + + renderConnectionPicker(modal, [r2], r2, null); + assert.equal( + wrap.classList.contains("hidden"), + true, + "hidden when there is nothing to choose", + ); +}); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index 62682d9d..85353db3 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -555,21 +555,34 @@ toward freshness and looked identical. Gave each a structural identity: the active signer if ever reused. ### 43. Logged-in profile card at the top of the profile modal (UX) -- [ ] Show the **logged-in user's profile photo + name** at the **top-center** of the - profile modal, styled as a "profile card" that sits **half-on / half-off** the - modal edge (overlapping the top border) for a polished look. Goal: users can see - WHICH profile they're acting as (e.g. uploading as) at a glance. Pull avatar/name - from the profile cache for the active pubkey; update on profile switch. +- [x] **DONE 2026-07-03.** An "Acting as " identity pill now renders + top-center on EVERY pane, straddling the header divider (negative + block-start margin — the modal is full-screen, so the header edge is the + overlap surface). Avatar + name come from the same resolveMeta path as + the Account pane card (profile cache first), populated in + renderSavedProfiles so it refreshes on every profile switch; hidden when + logged out; clicking it jumps to the Account pane. Markup in + components/profile-modal.html, styles in css/tailwind.source.css + (.profile-modal__identity*), wiring in profileModalController. Test in + tests/profile-modal-controller.test.mjs (26 pass). Lint + build green. +- [ ] **VERIFY on unstable:** open any pane → pill shows the active identity; + switch accounts → it updates; log out → it disappears; click → Account. ### 44. In-modal storage selector for uploads (UX) -- [ ] Make storage unlock + selection easier from the **video upload modal**: let the - user **pick any configured storage connection** (R2 / B2 / Custom S3) and unlock it - **without leaving the upload modal** — today they must close it, open Profile → - Storage, change the default/selected connection, then reopen upload. Add a provider - picker (driven by `storageService.listConnections`) + inline unlock in the upload - modal's storage section; selecting one sets it as the active upload target for this - upload (and optionally the default). Reuse `loadFromStorage` / the per-provider - connection model. +- [x] **DONE 2026-07-03.** The upload modal's storage summary now has an + "Upload destination"