From 24a34a373f25caddcf9d74830304baa2378ec0fd Mon Sep 17 00:00:00 2001 From: archipelago Date: Thu, 6 Aug 2026 12:11:57 -0400 Subject: [PATCH] fix(ui): tx links stop leaking to a third-party explorer on a load race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recurring regression (reported again on .228, 2026-08-06): clicking a tx opened the tx1138.com consent modal even though the node runs Mempool. Root cause was never the preference — getAppState() reports 'not-installed' for an app whose container list simply has not been fetched yet, so a click that landed before the list arrived took the external path. Timing-dependent, hence 'fixed a thousand times'. - container store: flag + (shared in-flight promise) so 'not yet known' is distinguishable from 'not installed'. - openTx: awaits real data, and the local app wins whenever installed — including stopped/restarting, where the app session's own controls are the right landing place. Only a genuinely app-less node goes external. - 5 regression tests incl. the race itself; vue-tsc -b clean. Co-Authored-By: Claude Fable 5 --- .../__tests__/useTxExplorer.test.ts | 86 +++++++++++++++++++ neode-ui/src/composables/useTxExplorer.ts | 22 ++++- neode-ui/src/stores/container.ts | 25 ++++++ 3 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 neode-ui/src/composables/__tests__/useTxExplorer.test.ts diff --git a/neode-ui/src/composables/__tests__/useTxExplorer.test.ts b/neode-ui/src/composables/__tests__/useTxExplorer.test.ts new file mode 100644 index 00000000..34751d6d --- /dev/null +++ b/neode-ui/src/composables/__tests__/useTxExplorer.test.ts @@ -0,0 +1,86 @@ +// Regression suite for the recurring "tx link opens tx1138.com instead of +// the local Mempool app" bug (reported again on .228, 2026-08-06). +// +// The root cause was never the explorer preference — it was that +// `getAppState` reports `not-installed` for an app whose container list has +// not been fetched yet. A click that landed before the list arrived sent +// the user to a third-party explorer, telling that operator which +// transaction they cared about. These tests pin the fix: the decision waits +// for real data, and the local app wins whenever it exists. +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { setActivePinia, createPinia } from 'pinia' + +const openSession = vi.fn() +vi.mock('@/stores/appLauncher', () => ({ + useAppLauncherStore: () => ({ openSession }), +})) + +let containerState: string +let fetched: boolean +const ensureFetched = vi.fn(async () => { + // Mirrors the real store: state only becomes knowable after the fetch. + fetched = true +}) +vi.mock('@/stores/container', () => ({ + useContainerStore: () => ({ + ensureFetched, + getAppState: (_id: string) => (fetched ? containerState : 'not-installed'), + }), +})) + +import { useTxExplorer, DEFAULT_TX_EXPLORER } from '../useTxExplorer' + +const TX = 'a'.repeat(64) + +describe('useTxExplorer.openTx', () => { + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + localStorage.clear() + fetched = false + containerState = 'running' + // Reset module-scope prefs/pending between tests. + const { setExplorer, cancelPending } = useTxExplorer() + setExplorer(DEFAULT_TX_EXPLORER, false) + cancelPending() + }) + + it('opens the local Mempool app when it is running', async () => { + const { openTx, pendingTx } = useTxExplorer() + await openTx(TX) + expect(openSession).toHaveBeenCalledWith('mempool', { path: `/tx/${TX}` }) + expect(pendingTx.value).toBeNull() + }) + + it('waits for the container list rather than assuming not-installed (the race)', async () => { + const { openTx, pendingTx } = useTxExplorer() + // fetched=false at click time — the old synchronous check read + // 'not-installed' here and went external. + await openTx(TX) + expect(ensureFetched).toHaveBeenCalled() + expect(openSession).toHaveBeenCalledWith('mempool', { path: `/tx/${TX}` }) + expect(pendingTx.value).toBeNull() + }) + + it('still prefers the local app when it is installed but stopped', async () => { + containerState = 'stopped' + const { openTx } = useTxExplorer() + await openTx(TX) + expect(openSession).toHaveBeenCalledWith('mempool', { path: `/tx/${TX}` }) + }) + + it('prefers the local app mid-restart rather than leaking to a third party', async () => { + containerState = 'restarting' + const { openTx } = useTxExplorer() + await openTx(TX) + expect(openSession).toHaveBeenCalledWith('mempool', { path: `/tx/${TX}` }) + }) + + it('asks for consent only when Mempool genuinely is not installed', async () => { + containerState = 'not-installed' + const { openTx, pendingTx } = useTxExplorer() + await openTx(TX) + expect(openSession).not.toHaveBeenCalled() + expect(pendingTx.value).toBe(TX) + }) +}) diff --git a/neode-ui/src/composables/useTxExplorer.ts b/neode-ui/src/composables/useTxExplorer.ts index efecc879..6359b97c 100644 --- a/neode-ui/src/composables/useTxExplorer.ts +++ b/neode-ui/src/composables/useTxExplorer.ts @@ -59,9 +59,25 @@ export function useTxExplorer() { window.open(`${explorerUrl()}/tx/${txHash}`, '_blank', 'noopener,noreferrer') } - /** Entry point for every "view transaction" affordance in the app. */ - function openTx(txHash: string) { - if (containers.getAppState('mempool') === 'running') { + /** + * Entry point for every "view transaction" affordance in the app. + * + * The local Mempool app WINS whenever it is installed — including while + * it is stopped or restarting, where the app session's own controls are + * the right place to land. Only a node that genuinely does not have the + * app (the pruned-node case this file was written for) ever reaches an + * external explorer. + * + * The await is load-bearing, not incidental. `getAppState` reports + * `not-installed` for an app it simply has not fetched yet, so the old + * synchronous check sent a user with a perfectly healthy local Mempool + * to a third-party explorer whenever they clicked before the container + * list arrived — a privacy leak decided by a race, and the reason this + * regression kept coming back (reported again on .228, 2026-08-06). + */ + async function openTx(txHash: string) { + await containers.ensureFetched() + if (containers.getAppState('mempool') !== 'not-installed') { launcher.openSession('mempool', { path: `/tx/${txHash}` }) return } diff --git a/neode-ui/src/stores/container.ts b/neode-ui/src/stores/container.ts index 5408f430..7211250f 100644 --- a/neode-ui/src/stores/container.ts +++ b/neode-ui/src/stores/container.ts @@ -95,6 +95,10 @@ export const useContainerStore = defineStore('container', () => { const containers = ref([]) const healthStatus = ref>({}) const loading = ref(false) + /** Whether the container list has been successfully fetched at least + * once. Without this, an empty list is ambiguous — see `ensureFetched`. */ + const fetched = ref(false) + let inFlightFetch: Promise | null = null const loadingApps = ref>(new Set()) // Track loading state per app const error = ref(null) @@ -198,6 +202,7 @@ export const useContainerStore = defineStore('container', () => { error.value = null try { containers.value = await containerClient.listContainers() + fetched.value = true } catch (e) { error.value = e instanceof Error ? e.message : 'Failed to fetch containers' if (import.meta.env.DEV) console.error('Failed to fetch containers:', e) @@ -206,6 +211,24 @@ export const useContainerStore = defineStore('container', () => { } } + /** + * Resolve the container list ONCE before a decision that depends on + * whether an app exists. `getAppState` cannot distinguish "not installed" + * from "not fetched yet" — both look like an empty list — so any caller + * that would take a DIFFERENT, user-visible path on "not installed" + * (tx links falling back to a third-party explorer, for one) must await + * this first. Concurrent callers share the one in-flight request. + */ + async function ensureFetched(): Promise { + if (fetched.value) return + if (!inFlightFetch) { + inFlightFetch = fetchContainers().finally(() => { + inFlightFetch = null + }) + } + await inFlightFetch + } + async function fetchHealthStatus() { try { healthStatus.value = await containerClient.getHealthStatus() @@ -354,6 +377,8 @@ export const useContainerStore = defineStore('container', () => { getAppVisualState, enrichedBundledApps, // Actions + fetched, + ensureFetched, fetchContainers, fetchHealthStatus, installApp,