fix(ui): tx links stop leaking to a third-party explorer on a load race
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
02a5aa6e29
commit
24a34a373f
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -95,6 +95,10 @@ export const useContainerStore = defineStore('container', () => {
|
||||
const containers = ref<ContainerStatus[]>([])
|
||||
const healthStatus = ref<Record<string, string>>({})
|
||||
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<void> | null = null
|
||||
const loadingApps = ref<Set<string>>(new Set()) // Track loading state per app
|
||||
const error = ref<string | null>(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<void> {
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user