mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-11-04 11:52:25 +00:00
docs: update zap-audit plan, pre-launch TODO, and test-integrity notes
Record this session's fixes (delete/zombie, NIP-71 mirror teardown, watch-history bucketing, the zap chain, the test-runner quarantine work) and the test spec-corrections. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
f7f246835a
commit
380de0922a
3 files changed
+481
-43
No files matched your search
@@ -99,3 +99,249 @@ test_integrity_note:
|
||||
backstop) rather than weakening a real correctness check; coverage of the
|
||||
actual security/decrypt behavior is preserved."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
```yaml
|
||||
test_integrity_note:
|
||||
date: 2026-06-23
|
||||
file: tests/nip71-mirror-flags.test.mjs
|
||||
change_type: ["spec_correction"]
|
||||
scenarios:
|
||||
- id: SCN-mirror-delete-unconditional
|
||||
given: "A video that was shared to NIP-71 on one device/session, then deleted from another device (or after the per-video opt-in flag — browser-local localStorage — was cleared)"
|
||||
when: "resolveDeleteSync runs during delete with FEATURE_NIP71_MIRROR on but the local enabled flag false/absent"
|
||||
then: "The mirror teardown (NIP-09 + empty tombstone, idempotent) is still attempted, so the NIP-71 mirror is not orphaned on other apps; the feature flag still fully gates it"
|
||||
observable_outcomes:
|
||||
- "resolveDeleteSync({featureOn:true, enabled:false|undefined}) -> {action:'unshare'}"
|
||||
- "resolveDeleteSync({featureOn:false, ...}) -> {action:'none'} (feature flag still gates)"
|
||||
determinism_controls:
|
||||
- "pure function; explicit input matrix"
|
||||
anti_cheat_rationale:
|
||||
prevents:
|
||||
- "hard-coded return value"
|
||||
note: "The previous expectation (enabled:false -> none) encoded the bug: it
|
||||
gated teardown on a browser-local flag, orphaning the mirror across devices.
|
||||
The corrected spec is STRICTER — teardown fires whenever the feature is on —
|
||||
and adds the enabled:undefined case. The feature-flag gate assertion is
|
||||
retained, so no real gate was weakened."
|
||||
relaxation:
|
||||
did_relax_any_assertion: false
|
||||
if_true_explain_spec_basis: ""
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
```yaml
|
||||
test_integrity_note:
|
||||
date: 2026-06-23
|
||||
file: tests/nostr-delete-flow.test.mjs
|
||||
change_type: ["spec_correction", "refactor_tests"]
|
||||
scenarios:
|
||||
- id: SCN-delete-surfaces-relay-failures
|
||||
given: "A logged-in delete where one write relay rejects the kind-5 deletion"
|
||||
when: "deleteAllVersions publishes to the WRITE set via getDeletePublishRelays() (the afb6200b relay-scope fix)"
|
||||
then: "The delete summary surfaces the failing relay"
|
||||
- id: SCN-buffered-feed-tombstone-guard
|
||||
given: "A delete event seeds a tombstone, then an OLDER non-deleted event for the same root arrives on the live stream"
|
||||
when: "events flow through the real VideoEventBuffer (the path subscribeVideos builds via the SubscriptionManager), not the removed direct pool.sub() injection"
|
||||
then: "The older event is tombstone-guarded (marked deleted, kept out of activeMap) and never surfaces as a LIVE video; a deletion event surfaces marked deleted:true so the UI removes the card"
|
||||
observable_outcomes:
|
||||
- "deleteSummary.summary.failed identifies wss://relay.fail (writeRelays now configured so the injected failure is exercised)"
|
||||
- "no callback video with the deleted/older event id and deleted:false"
|
||||
- "client.activeMap cleared for the root; tombstone advanced; stored event marked deleted"
|
||||
determinism_controls:
|
||||
- "videoEventVerifier virtualizes the off-thread signature worker (all events valid)"
|
||||
- "buffer.scheduleFlush(true) awaited for synchronous commit"
|
||||
- "real NostrClient + real VideoEventBuffer (no mocked tombstone guard)"
|
||||
anti_cheat_rationale:
|
||||
prevents:
|
||||
- "over-mocking internal logic"
|
||||
- "snapshot rubber-stamping"
|
||||
note: "Two corrections. (1) The file used bare assert + top-level await, so the
|
||||
unit runner (which requires a node:test import) SILENTLY EXCLUDED it — it had
|
||||
rotted and was failing on main unnoticed. Converted to node:test so it runs
|
||||
in CI. (2) The relay-failure test set only client.relays, but deletes moved to
|
||||
the write set (getDeletePublishRelays); it now sets writeRelays so the
|
||||
injected failure is actually exercised — the assertion was previously passing
|
||||
vacuously. (3) The subscription assertions drove the REMOVED pool.sub() path
|
||||
and expected 'no callback at all' for deleted events; the current
|
||||
VideoEventBuffer intentionally surfaces deleted events marked deleted:true so
|
||||
the UI can remove the card. Rewritten to drive the real buffer + real client
|
||||
and assert the stricter, accurate invariant (never surfaces as LIVE; guarded
|
||||
and removed from activeMap). Coverage is stronger: the buffer's own unit test
|
||||
mocks the client's tombstone guard, whereas this exercises the real guard."
|
||||
relaxation:
|
||||
did_relax_any_assertion: false
|
||||
if_true_explain_spec_basis: ""
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
```yaml
|
||||
test_integrity_note:
|
||||
date: 2026-06-23
|
||||
file: tests/watch-history.test.mjs
|
||||
change_type: ["spec_correction"]
|
||||
scenarios:
|
||||
- id: SCN-watch-history-partial-acceptance-durable
|
||||
given: "A logged-in user snapshots watch history to 3 write relays where 2 accept and 1 rejects (single attempt, no escalation)"
|
||||
when: "watchHistoryService.snapshot runs after the 565a9618 fix (a snapshot is durable as soon as ANY relay accepts; reads take the newest event per d-tag)"
|
||||
then: "The snapshot SUCCEEDS — it does not throw, the result is ok and not retryable, no republish is scheduled, the month result is flagged partial with the accepting/rejecting relays captured, and the pointer queue is emptied (the pointer was durably published)"
|
||||
- id: SCN-watch-history-logged-in-not-session
|
||||
given: "A logged-in user (nostrClient.pubkey set) records a watch and the entry is published and re-loaded"
|
||||
when: "publishView resolves the owning actor as the logged-in pubkey and sets the session flag only when nobody is logged in"
|
||||
then: "The round-tripped entry is NOT flagged session=true (it is the logged-in user's history, not a local-only session entry); the queue files under the logged-in pubkey so getQueuedPointers(actor) sees it"
|
||||
observable_outcomes:
|
||||
- "snapshot() resolves (does not throw); result.ok === true; result.retryable === false"
|
||||
- "scheduleWatchHistoryRepublish NOT called"
|
||||
- "result.results[0].partial === true; relayStatus.pointer shows relay.one/two accepted, relay.three rejected"
|
||||
- "getQueuedPointers(actor).length === 0 after a successful snapshot"
|
||||
- "resolvedItems[0].session !== true for a logged-in user"
|
||||
determinism_controls:
|
||||
- "poolHarness resolver with a fixed accept/reject plan per relay url"
|
||||
- "session crypto + recordVideoView virtualized; stable relay set"
|
||||
- "logged-in pubkey aligned with the queried actor (matches the convention used by the other tests in the file)"
|
||||
anti_cheat_rationale:
|
||||
prevents:
|
||||
- "snapshot rubber-stamping"
|
||||
- "hard-coded return value"
|
||||
note: "Three corrections in this file, all matching shipped behavior. (1) The
|
||||
partial-acceptance test asserted the INVERSE of intended behavior — that
|
||||
snapshot() THROWS on partial acceptance and retries until EVERY relay accepts —
|
||||
encoding exactly the all-relays false-failure bug 565a9618 fixed. Rewritten to
|
||||
the success path (no throw, partial flag, per-relay status, queue drained, no
|
||||
spurious republish). (2) testWatchHistoryServiceIntegration set the logged-in
|
||||
pubkey to a value DIFFERENT from the actor it queried; since watch history now
|
||||
files under the logged-in pubkey, the queue assertions looked at the wrong key.
|
||||
Aligned pubkey to the queried actor (the convention every other test uses) — a
|
||||
setup fix, no assertion weakened. (3) The same test asserted session===true for
|
||||
a logged-in watch, which is the pre-fix bug (logged-in watches were wrongly
|
||||
flagged session by comparing against the view actor); corrected to session!==true
|
||||
per the current rule that session marks only logged-out, local-only entries.
|
||||
With these the full file is green and was REMOVED from the run-unit-tests
|
||||
QUARANTINE."
|
||||
relaxation:
|
||||
did_relax_any_assertion: false
|
||||
if_true_explain_spec_basis: ""
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
```yaml
|
||||
test_integrity_note:
|
||||
date: 2026-06-23
|
||||
files:
|
||||
- tests/zap-shared-state.test.mjs
|
||||
- tests/video-modal-zap.test.mjs
|
||||
change_type: ["spec_correction", "refactor_tests"]
|
||||
scenarios:
|
||||
- id: SCN-platform-fee-unparseable-override
|
||||
given: "An unparseable platform-fee override value (e.g. 'not-a-number') with PLATFORM_FEE_PERCENT configured > 0"
|
||||
when: "resolvePlatformFeePercent runs"
|
||||
then: "It falls back to the configured platform default (getDefaultPlatformFeePercent), NOT to 0 — a junk override cannot silently disable/bypass the platform fee"
|
||||
- id: SCN-zap-completion-state-resets-then-completes
|
||||
given: "A zap whose receipts validate to a warning (awaiting validated receipt)"
|
||||
when: "sendZap runs"
|
||||
then: "The modal completion state is reset to false at the start of the send and set true at the terminal path, so two completion-state changes are recorded and the end state is true"
|
||||
- id: SCN-zap-receipts-render-into-controller-element
|
||||
given: "VideoModal.renderZapReceipts called with one validated (or one failed) receipt"
|
||||
when: "rendering delegates to the inner zapController"
|
||||
then: "Exactly one entry is rendered into the controller's modalZapReceipts element"
|
||||
observable_outcomes:
|
||||
- "resolvePlatformFeePercent('not-a-number') === getDefaultPlatformFeePercent() (> 0)"
|
||||
- "completedStates grows by 2 (false then true) on a completing send; end state true"
|
||||
- "controller.modalZapReceipts.children.length === 1 per rendered receipt"
|
||||
determinism_controls:
|
||||
- "pure fee function with explicit inputs; configured PLATFORM_FEE_PERCENT read via getDefaultPlatformFeePercent (not hard-coded)"
|
||||
- "modal stub records setZapCompleted/resetZapForm calls; DummyElement DOM"
|
||||
anti_cheat_rationale:
|
||||
prevents:
|
||||
- "hard-coded return value"
|
||||
- "over-mocking internal logic"
|
||||
note: "Three corrections, all to match shipped behavior. (1) The fee-fallback test
|
||||
asserted 0; the function correctly falls back to the configured platform default
|
||||
so a garbage override cannot zero out (bypass) the fee — corrected to assert
|
||||
against getDefaultPlatformFeePercent and that the result is > 0 (STRICTER: pins
|
||||
the no-bypass guarantee). (2) The completion-count test asserted +1; sendZap
|
||||
legitimately resets completed=false at the start of every send and sets true at
|
||||
the terminal path (+2), with the end-state==true assertion retained as the real
|
||||
invariant. (3) The receipt-render tests set modalZapReceipts on the outer
|
||||
VideoModal, but rendering was refactored to delegate to the inner zapController
|
||||
which owns that element — set/assert on the controller's element (a harness fix,
|
||||
no assertion weakened). NOTE: these accompany three PRODUCTION bug fixes found
|
||||
via the same tests — NWC getPublicKey hex->bytes (connection was broken), the
|
||||
in-flight 'Sending…' status mis-toned as warning, and the success path not using
|
||||
the holistic resetZapForm."
|
||||
relaxation:
|
||||
did_relax_any_assertion: false
|
||||
if_true_explain_spec_basis: ""
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
```yaml
|
||||
test_integrity_note:
|
||||
date: 2026-06-23
|
||||
file: tests/video-modal-zap.test.mjs
|
||||
change_type: ["spec_correction"]
|
||||
scenarios:
|
||||
- id: SCN-zap-paid-but-receipt-unconfirmed-is-success
|
||||
given: "A zap share whose PAYMENT succeeded (NWC returned a preimage) but whose kind-9735 zap receipt could not be validated on the advertised relays"
|
||||
when: "sendZap finishes"
|
||||
then: "It is reported as a SUCCESS (Sent N sats) with a soft note that the on-relay receipt couldn't be confirmed — NOT as a warning/error. The form resets and the attempt is marked completed."
|
||||
observable_outcomes:
|
||||
- "last zap status tone === 'success' (was 'warning')"
|
||||
- "status message matches /sent 750 sats/i and /couldn't confirm the zap receipt/i (was /awaiting validated zap receipt/i)"
|
||||
- "receipts rendered in partial mode; form reset; completedStates +2 ending true"
|
||||
determinism_controls:
|
||||
- "injected splitAndZap returns a paid receipt (status:success, payment.preimage) with validation.status:'failed'"
|
||||
anti_cheat_rationale:
|
||||
prevents:
|
||||
- "snapshot rubber-stamping"
|
||||
- "hard-coded return value"
|
||||
note: "The previous expectation asserted the BUG: a paid share whose 9735 receipt
|
||||
wasn't found was surfaced as a 'warning' error ('Awaiting validated zap receipt')
|
||||
and routed through notifyError — telling users a SUCCESSFUL zap had failed
|
||||
(confirmed live: the platform fee landed but the UI showed a red error). The NWC
|
||||
payment preimage is proof the invoice was paid; the 9735 receipt is supplementary
|
||||
and is commonly not seen by a static client (relay coverage/timing). Corrected to
|
||||
assert success-with-a-soft-note. Genuine PAYMENT failures (status !== 'success')
|
||||
still go through the failure/retry path, unchanged."
|
||||
relaxation:
|
||||
did_relax_any_assertion: false
|
||||
if_true_explain_spec_basis: ""
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
```yaml
|
||||
test_integrity_note:
|
||||
date: 2026-06-23
|
||||
file: tests/zap-split.test.mjs
|
||||
change_type: ["spec_correction"]
|
||||
scenarios:
|
||||
- id: SCN-zap-receipt-found-by-indexed-tags
|
||||
given: "A published kind-9735 zap receipt for a video zap (the zap request carries #p recipient + #e event tags)"
|
||||
when: "validateZapReceipt queries relays for the receipt"
|
||||
then: "It filters by the reliably-indexed #e + #p tags (not #bolt11, which most relays don't index), then confirms via author pubkey + bolt11 + description hash"
|
||||
observable_outcomes:
|
||||
- "the relay query filter has filters[0]['#e'] === [eventId] and ['#p'] === [recipient]"
|
||||
- "filters[0]['#bolt11'] is absent"
|
||||
- "a matching receipt still validates as status:'passed'"
|
||||
determinism_controls:
|
||||
- "mock SimplePool.list captures the filter; fixed zap request tags + test bolt11 with matching description hash"
|
||||
anti_cheat_rationale:
|
||||
prevents:
|
||||
- "snapshot rubber-stamping"
|
||||
- "over-mocking internal logic"
|
||||
note: "The previous test asserted filters[0]['#bolt11'][0] === invoice — encoding the
|
||||
bug. Relays generally do not index arbitrary tags like bolt11, so that query found
|
||||
nothing and a successfully-paid zap was reported as having no receipt. Corrected to
|
||||
assert the reliably-indexed #e/#p anchors (added an #e tag to the request to model a
|
||||
real video zap). The receipt-matching assertions (passed status, correct event,
|
||||
checked relays) are unchanged/retained — coverage is stronger, not weaker."
|
||||
relaxation:
|
||||
did_relax_any_assertion: false
|
||||
if_true_explain_spec_basis: ""
|
||||
```
|
||||
+91
-7
@@ -18,6 +18,88 @@ Pre-launch priority #3. The zap flow feels clunky and errors on send.
|
||||
platform address via `platformAddress.js` / `PLATFORM_LUD16_OVERRIDE`.
|
||||
|
||||
## Findings so far
|
||||
|
||||
### 2026-06-23 session 3 (live-test: successful zap reported as error)
|
||||
- **A successful zap was shown as a red error (FIXED, item #4).** Live test: zapping
|
||||
`[email protected]` (CORS-friendly) actually PAID — the platform fee landed — but
|
||||
bitvid showed *"Awaiting validated zap receipt… No zap receipt was published on the
|
||||
advertised relays."* Root cause: a share whose PAYMENT succeeded (NWC returned a
|
||||
preimage) but whose kind-9735 receipt couldn't be validated was routed through
|
||||
`notifyError` as a warning. The preimage is proof the invoice was paid; the 9735 is
|
||||
supplementary and a static client commonly can't see it (relay coverage/timing/late
|
||||
publish). Fixed (`js/ui/zapController.js`): paid-but-unvalidated is now a SUCCESS with
|
||||
a soft note ("Payment went through; couldn't confirm the zap receipt on relays").
|
||||
Genuine payment failures (status !== success) still go through the failure/retry path.
|
||||
Spec-corrected test in `tests/video-modal-zap.test.mjs` (see TEST_INTEGRITY.md).
|
||||
RECEIPT DISCOVERY FIXED (item #4, 2026-06-23): the validator queried receipts with
|
||||
`{ kinds:[9735], "#bolt11":[invoice] }` — but most relays DON'T index arbitrary tags
|
||||
like `bolt11`, so the query returned nothing even when the receipt was published.
|
||||
New `buildReceiptFilters` (`js/payments/zapReceiptValidator.js`) queries by the
|
||||
reliably-indexed `#e` (event) / `#a` (coordinate) + `#p` (recipient) tags from the
|
||||
zap request, falling back to `#bolt11` only if no anchor exists; the precise match
|
||||
(author pubkey + bolt11 + description hash) still confirms the right receipt. Tests
|
||||
in `tests/zap-receipt-pool.test.mjs` + spec-corrected `tests/zap-split.test.mjs`.
|
||||
TIMING GAP CLOSED (2026-06-23): the validator now POLLS for the receipt — up to 3
|
||||
lookups (default), ~1.2s apart — so a receipt the recipient's service publishes a
|
||||
few seconds after EOSE is still caught. Stops as soon as it's found (no delay in the
|
||||
common case); attempts/delay/sleep are injectable (`receiptLookupAttempts`,
|
||||
`receiptLookupDelayMs`, `sleep`) and tests stub `sleep` so they don't wait. Reason
|
||||
now distinguishes "no events at all" vs "events but none matched". Tests:
|
||||
`testValidateZapReceiptRetriesForLateReceipt` (found on 3rd poll) +
|
||||
`testValidateZapReceiptGivesUpAfterRetries` in `tests/zap-split.test.mjs`. NOTE: in
|
||||
the no-receipt case this adds ~2.4s before the (success-with-note) status shows;
|
||||
acceptable for a payment confirmation. A fully background validate-and-upgrade flow
|
||||
remains a possible future refinement.
|
||||
- LOG NOISE seen during the live test (pre-existing, separate items): cold-start
|
||||
RELAY STORM (~28 REQ/s, item #9); `bad req: uneven size input to from_hex` (odd-length
|
||||
hex reaching a relay — the sanitizer #9 should catch this; re-confirm); watch-history
|
||||
bucketing items with `watchedAt:0` into a `1970-01` month (minor watch-history bug).
|
||||
|
||||
### 2026-06-23 session 2 (live-test fixes)
|
||||
- **Couldn't type in the zap comment box (FIXED, real bug).** The shared popover
|
||||
engine treats every panel as an ARIA menu and `preventDefault`'d single-character
|
||||
+ arrow keydowns → every keystroke in the comment/amount fields was swallowed.
|
||||
`handleMenuKeydown` now ignores events from editable targets
|
||||
(input/textarea/select/contenteditable/role=textbox). Regression test in
|
||||
`tests/ui/popoverEngine.test.mjs`.
|
||||
- **NWC wallet didn't restore "like storage" (FIXED, real bug).** The "already
|
||||
offered" flag in `settingsRestorePrompt.js` was per-PUBKEY, so the first item
|
||||
offered (storage) flagged the whole account and the wallet note was never offered
|
||||
afterward. Now tracked PER-ITEM. Decision (user, 2026-06-23): keep the one-time
|
||||
CONFIRM for the wallet (it's a spending secret) — not silent auto-restore.
|
||||
Regression test in `tests/settings-restore-prompt.test.mjs`.
|
||||
- **"Click the Zap button many times before it opens" (FIX — needs live confirm).**
|
||||
A click during an in-flight open canceled it (`pendingToggle="close"`), so rapid
|
||||
double-clicks toggled the popover back closed. Now an in-flight-open click is
|
||||
ignored unless the dialog is already visibly open. (`video-modal/zapController.js`)
|
||||
- **CORS confirmed for goblinbox** (`unitypay.cash/.well-known/lnurlp/goblinbox` →
|
||||
net::ERR_FAILED). With the NWC fix in place, `wallet.hasUri:true` — the send fails
|
||||
purely at the LNURL fetch (CORS), not NWC. This is the data point for the proxy
|
||||
decision (item 1).
|
||||
|
||||
### 2026-06-23 session (fixes landed)
|
||||
- **NWC wallet connection was BROKEN (launch-blocker, FIXED).** `parseNwcUri`
|
||||
(`js/payments/nwcClient.js`) passed the hex `secret` to nostr-tools `getPublicKey`,
|
||||
which in v2.x requires a `Uint8Array` → it threw `expected Uint8Array` and the whole
|
||||
NWC connect failed (NWC is the only zap payment path). Now converts hex→bytes (try
|
||||
hex, fall back to `hexToBytesCompat`). Real-tools regression test:
|
||||
`tests/nwc-parse-uri.test.mjs`. (The mock-based `tests/nwc-client.test.mjs` is still
|
||||
quarantined — its nostr-tools mock is shadowed by the frozen canonical toolkit; needs
|
||||
a harness rework, but the bug itself is fixed + guarded.)
|
||||
- **In-flight "Sending… sats" status was mis-toned as `warning` (FIXED).** A pending
|
||||
state was colored `text-warning-strong` (alarming) and counted as a warning. Now
|
||||
`neutral`. (`js/ui/zapController.js`)
|
||||
- **Success path didn't fully reset the form (FIXED).** The terminal success path
|
||||
cleared only the comment (`setZapComment`) while the validation-warning path used the
|
||||
holistic `resetZapForm`. Unified to `resetZapForm({amount:"",comment:""})` so amount +
|
||||
comment reset consistently after a send. (`js/ui/zapController.js`)
|
||||
- **Fee-fallback is no-bypass (verified).** `resolvePlatformFeePercent` falls back to
|
||||
the configured `PLATFORM_FEE_PERCENT` (30) on an unparseable override — a junk override
|
||||
can't silently disable the fee. (Stale test that expected 0 corrected.)
|
||||
- **3a popover** — FIXED separately (see TODO 3a); the in-modal zap dialog now portals
|
||||
to `#uiOverlay` so it positions `bottom-end` under the button.
|
||||
|
||||
### Earlier findings
|
||||
- **Send error = recipient LNURL unreachable (CORS/offline).** [PARTLY FIXED]
|
||||
Sending to a creator whose LNURL host is down or lacks CORS headers (observed:
|
||||
`unitypay.cash/.well-known/lnurlp/goblinbox`) makes the browser `fetch` reject
|
||||
@@ -31,13 +113,15 @@ Pre-launch priority #3. The zap flow feels clunky and errors on send.
|
||||
- **Popover mis-positioning (zap + embed)** — see TODO 3a; shared `popoverEngine`.
|
||||
|
||||
## To do (next session, in priority order)
|
||||
1. **CORS / LNURL proxy decision (the real reliability fix).** A static client
|
||||
can't fetch LNURL endpoints that omit CORS headers, so zaps to many creators
|
||||
fail. Options: (a) a small server-side/edge proxy (Vercel function) that fetches
|
||||
the LNURL pay-service data + invoice and returns it with CORS — what most web
|
||||
wallets do; (b) document the limitation and only support CORS-enabled hosts;
|
||||
(c) route via the connected NWC wallet if it can resolve LN addresses. Decide,
|
||||
then implement. Confirm whether `unitypay.cash` is also just offline.
|
||||
1. **CORS / LNURL proxy — DECISION: re-test first (2026-06-23).** Because the NWC
|
||||
connection was fully broken (hex secret → `getPublicKey`; fixed this session), an
|
||||
unknown share of the observed "send errors" were the NWC bug, NOT CORS. Plan:
|
||||
deploy the NWC fix, run real zaps, and measure the actual CORS failure rate before
|
||||
committing to a proxy. THEN choose: (a) small Vercel edge proxy that fetches LNURL
|
||||
pay-data + invoice and returns with CORS [recommended if the rate is high — what web
|
||||
wallets do]; (b) document the limit / CORS-only hosts; (c) route LN-address
|
||||
resolution via the connected NWC wallet. Also confirm whether `unitypay.cash` is
|
||||
simply offline vs. CORS-less.
|
||||
2. **Popover positioning (3a).** Fix the shared engine safely (portal in-modal
|
||||
panels out to `#uiOverlay`, or remove the modal-subtree containing block).
|
||||
Needs the live computed `position/left/top` + DOM parent of `#modalZapDialog`.
|
||||
|
||||
@@ -36,17 +36,37 @@ tests → `npm run build` + `npm run test:unit` green → commit + push.
|
||||
the subset kept serving the original → resurrection. Now both delete paths
|
||||
(soft-delete tombstone + NIP-09) use `getDeletePublishRelays()` (write set).
|
||||
Tombstone created_at was already bumped strictly-newer, so this closes the gap.
|
||||
- [ ] **Verify with the user** that zombies are gone after this fix. If any remain,
|
||||
remaining suspects to audit:
|
||||
- Is the local cache / `videosMap` + persisted `bitvid:filtered-videos:v1`
|
||||
purged on delete, and does the feed filter honor `deleted` + tombstones on
|
||||
an optimistic (cache-first) reload?
|
||||
- Are NIP-71 (kind 22) + NIP-94 (kind 1063) mirrors also tombstoned, or can a
|
||||
lingering mirror resurrect the card?
|
||||
- getActiveKey mismatch between original (pubkey:dTag) and tombstone (ROOT:…)
|
||||
for edge-case legacy videos.
|
||||
- [ ] Add a feed-level regression test (seed event → delete → assert not rendered)
|
||||
once the cache/mirror angles above are confirmed.
|
||||
- [x] **getActiveKey mismatch — ROOT-CAUSED + FIXED (2026-06-23).** `js/nostr/client.js`
|
||||
carried a STALE local duplicate of `getActiveKey` (added before the LEGACY guard
|
||||
landed in `js/nostr/utils.js` at `1b11cb1b`). The tombstone machinery
|
||||
(`recordTombstone`/`isOlderThanTombstone`/`applyTombstoneGuard`) used the local
|
||||
copy, so a deleted legacy video (synthesized `videoRootId = LEGACY:<pubkey>:<dTag>`)
|
||||
keyed its tombstone as `ROOT:LEGACY:…` while a bare zombie from a relay keyed as
|
||||
`<pubkey>:<dTag>` → guard never matched → resurrection. Fix: client.js now imports
|
||||
the single canonical `getActiveKey` from utils.js (duplicate deleted). Regression
|
||||
test added in `tests/nostr/client.test.mjs` ("tombstone guard (legacy zombie
|
||||
suppression)") — in the CI suite.
|
||||
- [x] **Cache / optimistic-reload angle — audited, OK.** `js/nostr/videoEventBuffer.js`
|
||||
checks tombstones + `deleted` on every buffered event (cache-first reload honors
|
||||
deletions; lines ~89, 156–172).
|
||||
- [x] **Mirror-resurrection angle — audited + GAP FIXED (2026-06-23).**
|
||||
`js/services/nip71MirrorSync.js` listens on `videos:deleted` →
|
||||
`nip71MirrorService.remove()` (NIP-09 on BOTH addressable kinds 34235/34236 +
|
||||
empty-replace tombstone). Legacy auto-21/22 publisher stays dormant.
|
||||
GAP: teardown was gated on the per-video opt-in flag, which is **browser-local
|
||||
only** (`bitvid:nip71-mirror:v1`). A video shared on one device and deleted
|
||||
from another (or after a cache clear) skipped teardown → the NIP-71 mirror
|
||||
orphaned on other apps. Fixed in `resolveDeleteSync` (`js/services/
|
||||
nip71MirrorFlags.js`): delete now ALWAYS attempts teardown when
|
||||
FEATURE_NIP71_MIRROR is on, ignoring the local flag (remove() is idempotent —
|
||||
NIP-09 + empty tombstone are no-ops if no mirror exists). The flag still gates
|
||||
the UI toggle; the feature flag still fully gates publishing. Spec-corrected
|
||||
unit test + sync-level spy tests (cross-device teardown + best-effort no-throw)
|
||||
in `tests/nip71-mirror-flags.test.mjs` + `tests/nip71-mirror-sync.test.mjs`.
|
||||
- [ ] **Verify with the user** that zombies are gone on the live site after these fixes
|
||||
(the getActiveKey fix needs `npm run build` + redeploy to unstable to take effect).
|
||||
- [ ] (Nice-to-have) Add a feed/view-level seed→delete→assert-not-rendered test now that
|
||||
the client-level guard + buffer tombstone paths are covered by unit tests.
|
||||
|
||||
### 2. Watch history delete does not work at all
|
||||
- [x] **Root cause found + fixed** (`5f19f536`): the removal path called
|
||||
@@ -93,37 +113,68 @@ tests → `npm run build` + `npm run test:unit` green → commit + push.
|
||||
no-op) — the existing profile-modal-controller test file has an unusual
|
||||
wrapper, so add carefully — plus a feed/view-level seed→delete→assert test.
|
||||
(Local + publish/read layers are already unit+mutation tested.)
|
||||
- [x] **"1970-01" month bucketing FIXED (2026-06-23).** Legacy/migrated history items
|
||||
with no real `watchedAt` were siloed into a literal `1970-01` month event
|
||||
(kind 30079) that self-perpetuated (read back as 0 → re-bucketed → re-published
|
||||
with 0). `canonicalizeWatchHistoryItems` now derives a STABLE watch time from
|
||||
the pointer d-tag's embedded ms-timestamp (the video creation time) and
|
||||
backfills it, so those items land in a real month and the 0→1970 loop stops.
|
||||
Only truly undecodable pointers still use the epoch bucket. Helper
|
||||
`deriveWatchedAtFromPointer` + `tests/watch-history-bucketing.test.mjs`. NOTE:
|
||||
the old `1970-01` event already on relays is left as harmless stale data (not
|
||||
recreated); actively clearing it was declined as extra surgery.
|
||||
|
||||
### 3a. Video-modal popover mis-positioning (zap + embed) — BUG
|
||||
Confirmed via screenshots: the zap/embed popovers OPEN and are fully functional
|
||||
(comment box, amount, send all render) but appear anchored to the far LEFT of the
|
||||
content column instead of `bottom-end` under the button (which is in the right-
|
||||
side action bar). Both share `js/ui/overlay/popoverEngine.js`.
|
||||
- Leading cause: the engine only portals a panel into the body-level overlay root
|
||||
`if (!panel.isConnected)`. The zap/embed panels are pre-existing connected
|
||||
elements INSIDE the modal, so they're NOT portaled out, and `position:fixed`
|
||||
resolves against an in-modal containing block (something in the modal subtree
|
||||
has transform/filter/contain) → horizontal offset.
|
||||
- Ruled out: positioning CSS present in built css; floating-ui local bundle loads;
|
||||
panel class correct (`.popover__panel`/`.popover-panel` share the rule).
|
||||
- Fix (do carefully — shared engine drives every popover): either portal connected
|
||||
in-modal panels out to `#uiOverlay` on open (and restore/clean on close without
|
||||
losing the element for re-open), OR remove the containing-block ancestor in the
|
||||
modal subtree. VERIFY non-modal popovers (card "⋯" menu, feed-settings gear,
|
||||
FeedInfoPopover) still position correctly after the change.
|
||||
- To pin exactly: inspect `#modalZapDialog` while open — computed `position`,
|
||||
`left`, `top`, and its DOM parent (is it under `#uiOverlay` at body level or
|
||||
still nested in the modal?).
|
||||
### 3a. Video-modal popover mis-positioning (zap) — FIXED 2026-06-23
|
||||
The zap popover (`#modalZapDialog`) opened functional but anchored to the modal
|
||||
edge instead of `bottom-end` under its trigger.
|
||||
- [x] **Root cause confirmed + fixed** in the shared engine
|
||||
(`js/ui/overlay/popoverEngine.js`). The zap render fn returns the PRE-EXISTING
|
||||
in-modal `#modalZapDialog` verbatim (`() => this.modalZapDialog`,
|
||||
`zapController.js`). The engine only appended panels to the body-level portal
|
||||
`if (!panel.isConnected)`, so this already-connected panel was never moved
|
||||
out; its `position: fixed` then resolved against a transformed/contained modal
|
||||
ancestor → edge anchoring. Fix: the engine now `relocatePanelToPortal()` on
|
||||
open (records the panel's origin: parent + nextSibling, moves it into the
|
||||
`#uiOverlay` portal) and `restorePanelToOrigin()` on close (and via destroy),
|
||||
so the host DOM stays intact and re-open works. Fresh portal-owned panels
|
||||
(card "⋯" menu, share/more menus, feed-settings gear) are unaffected — they're
|
||||
already appended to the portal, so relocate is a no-op for them.
|
||||
Style-safe: `#modalZapDialog` uses the same `popover__panel card …` classes the
|
||||
already-portaled menus use; no `.modal`-descendant CSS. Regression test in
|
||||
`tests/ui/popoverEngine.test.mjs` ("relocates a pre-existing in-host panel …");
|
||||
all 9 engine tests + the menu-controller tests pass; build green.
|
||||
- [ ] **Verify with the user** on the live site after deploy that the zap popover
|
||||
now opens `bottom-end` under the Zap button (and re-opens correctly).
|
||||
- NOTE: the "embed" in the original report is a separate full modal
|
||||
(`components/embed-video-modal.html`, `#embedVideoModal`), NOT an anchored
|
||||
popover, so it isn't affected by this engine bug. The share/copy menu uses a
|
||||
fresh portaled panel and already positions correctly. If any other pre-existing
|
||||
in-modal panel is found mis-positioned, this same engine fix now covers it.
|
||||
|
||||
### 3. Zaps system + platform-fee zap split — full plan in docs/zap-audit-plan.md
|
||||
Audit started 2026-06-23. See the doc for architecture map + findings. Summary:
|
||||
- [x] **Send-error softened** (`dd1ce113`): recipient LNURL unreachable/CORS now
|
||||
shows a clear message, not raw "Failed to fetch".
|
||||
- [ ] **CORS / LNURL proxy decision** (the real reliability fix — static client
|
||||
can't fetch CORS-less LNURL hosts; many zaps fail). Top of the next session.
|
||||
- [ ] **Popover positioning (3a)**, **platform-fee split correctness** (incl. the
|
||||
earlier "fee landed in my own wallet" report + self-zap edge), **receipt
|
||||
validation (9735)**, **NWC budget/retry UX**, **general clunkiness**.
|
||||
- [x] **NWC connection bug FIXED (2026-06-23, launch-blocker)** — `parseNwcUri`
|
||||
passed a hex secret to nostr-tools `getPublicKey` (needs bytes) → NWC connect
|
||||
threw `expected Uint8Array` and failed entirely. Now hex→bytes. Guarded by
|
||||
`tests/nwc-parse-uri.test.mjs`. **This likely explains a lot of the "errors on
|
||||
send".** VERIFY live: connect a wallet + send a zap end-to-end.
|
||||
- [x] **Popover positioning (3a)** — FIXED (see 3a).
|
||||
- [x] **In-flight status + form-reset polish (2026-06-23)** — "Sending…" no longer
|
||||
mis-toned as a warning; success path fully resets the form (`resetZapForm`).
|
||||
- [x] **Platform-fee fallback verified no-bypass** — junk override falls back to the
|
||||
configured fee, not 0.
|
||||
- [ ] **CORS / LNURL proxy decision** (NEEDS A DECISION — the remaining reliability
|
||||
fix). A static client can't fetch CORS-less LNURL hosts; options: (a) small
|
||||
Vercel edge proxy that fetches LNURL pay-data + invoice and returns with CORS
|
||||
[recommended — what web wallets do]; (b) document the limit / CORS-only hosts;
|
||||
(c) route LN-address resolution via the connected NWC wallet. See
|
||||
docs/zap-audit-plan.md. NOTE: with NWC now connecting, re-test how many sends
|
||||
actually fail on CORS vs were just the NWC bug.
|
||||
- [ ] **Remaining audit items**: platform-fee split correctness (the earlier "fee
|
||||
landed in my own wallet" report + self-zap / creator==platform edge), receipt
|
||||
validation (9735), NWC budget/retry UX, general clunkiness.
|
||||
- [x] Comment box confirmed WORKING (the "message doesn't work" was the popover
|
||||
mis-position making it hard to use — see 3a).
|
||||
|
||||
@@ -213,6 +264,63 @@ un-hides when WebTorrent flips green). The real gaps:
|
||||
reproduced on pre-refactor `1b11cb1b`). Make it deterministic so it can be a
|
||||
trusted release gate. Audit e2e parallel-load flakiness too.
|
||||
|
||||
### 11b. SILENTLY-EXCLUDED unit tests — 20 files never run in CI (found 2026-06-23)
|
||||
`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
|
||||
referenced by no other CI script. They rot invisibly (this is how the delete-flow
|
||||
test below was failing on `main` unnoticed).
|
||||
- [x] **`tests/nostr-delete-flow.test.mjs` — FIXED + brought into CI** (converted to
|
||||
node:test; stale write-relay config + removed-pool.sub subscription harness
|
||||
corrected; see TEST_INTEGRITY.md 2026-06-23).
|
||||
- [ ] **Add a guard so this can't recur**: make `run-unit-tests.mjs` emit a visible
|
||||
WARNING (or fail) when it finds a `*.test.{mjs,js}` lacking a `node:test` import,
|
||||
listing each skipped file. (Decide warn-vs-fail — fail would immediately red-CI
|
||||
on the ~11 broken files below until they're triaged.)
|
||||
- [x] **Runner guard SHIPPED (2026-06-23).** `scripts/run-unit-tests.mjs` now collects
|
||||
EVERY `*.test.{mjs,js}` (no more node:test content filter) and runs both styles.
|
||||
Known-broken files are listed in an explicit `QUARANTINE` map — loudly reported
|
||||
on every run (never silently skipped), and a stale-entry check flags any
|
||||
quarantine path that no longer exists. The 10 PASSING orphans below now run in
|
||||
CI automatically (no per-file conversion needed).
|
||||
- PASS, now in CI: `nostr-view-event-bindings`, `nostr-rebroadcast-guard`,
|
||||
`watch-history-feed`, `subscriptions-feed`, `discussion-count-service`,
|
||||
`feed-engine`, `zap-split`, `nostr-view-events`,
|
||||
`watchHistory/watch-history-telemetry`, `unit/ui/thumbnailBinder`.
|
||||
- [ ] **Triage + un-quarantine the 10 broken files** (first-error diagnosis 2026-06-23;
|
||||
each needs the stale-vs-real-bug investigation the delete-flow file got):
|
||||
- `watch-history` (#2) — DONE + UN-QUARANTINED 2026-06-23. Three stale tests,
|
||||
all spec-corrected to shipped behavior (see TEST_INTEGRITY.md):
|
||||
(1) `testWatchHistoryPartialRelayRetry` asserted snapshot THROWS on partial
|
||||
acceptance (contradicting `565a9618`) → now asserts success / no-throw /
|
||||
no-republish / queue-emptied; (2) `testWatchHistoryServiceIntegration` set a
|
||||
logged-in pubkey different from the queried actor (history files under the
|
||||
logged-in pubkey now) → aligned; (3) same test asserted `session===true` for a
|
||||
logged-in watch (the pre-fix bug) → corrected to `session!==true`. The full
|
||||
2669-line file passes via the CI runner and was removed from QUARANTINE.
|
||||
- `nostr-boost-actions` — "Missing expected exception" (likely stale: expects a
|
||||
throw the code no longer makes).
|
||||
- `view-counter` (#4) — "view event should append the session tag when
|
||||
requested" (stale-or-real; investigate with #4).
|
||||
- `zap-shared-state` (#3) — ✅ DONE + UN-QUARANTINED 2026-06-23: stale fee-fallback
|
||||
test (expected 0; the function correctly falls back to the configured default so
|
||||
a junk override can't bypass the fee) — corrected.
|
||||
- `video-modal-zap` (#3) — ✅ DONE + UN-QUARANTINED 2026-06-23: surfaced TWO real
|
||||
zap bugs (in-flight status mis-toned `warning`→`neutral`; success path now uses
|
||||
the holistic `resetZapForm`) + spec/harness fixes (completion-count reset+complete;
|
||||
receipt render asserts on the inner zapController's element after the
|
||||
VideoModal→sub-controller refactor).
|
||||
- `nwc-client` (#3) — REAL production bug FIXED 2026-06-23: `parseNwcUri` passed a
|
||||
hex secret to nostr-tools `getPublicKey` (needs bytes) → NWC connection broken.
|
||||
Fixed + guarded by the real-tools `tests/nwc-parse-uri.test.mjs`. The mock-based
|
||||
`nwc-client.test.mjs` STAYS quarantined — its nostr-tools mock is shadowed by the
|
||||
frozen canonical toolkit the bootstrap installs; needs a mock-injection rework.
|
||||
- `nostr-count-fallback` — `TypeError: Cannot read 'has' of undefined` — likely
|
||||
a stale harness mock missing a field.
|
||||
- `admin-list-store` — community-blacklist merge mismatch.
|
||||
- `nostr-publish-rejection` — rejection-path assertion.
|
||||
- `user-blocks` — HANGS (async leak); needs a deterministic rewrite.
|
||||
|
||||
### 12. Promotion: `unstable → beta`
|
||||
- [ ] After this batch soaks and the high-priority items land, promote `unstable → beta`
|
||||
for hosted-domain QA (per the release pipeline in CLAUDE.md). Never push directly
|
||||
|
||||
Reference in new issue
Block a user