refactor(assets): derive flat-output loading flags from in-flight promises

Split fetchFlatOutputs into updateFlatOutputs / loadMoreFlatOutputs so each
path is independently readable and the "do not call directly" footgun goes
away. Replace the mirrored loading booleans with computeds over the in-flight
promise slots, removing the manually-synced duplicate state.

Swap flatOutputSeenIds on refresh success rather than before the request, so
a failed refresh leaves the loaded-id check consistent with the list still on
screen instead of re-appending the head page on the next loadMore.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: mattmillerai

src/stores/assetsStore.test.ts

@@ -1802,7 +1802,21 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
   })
 
   describe('in-flight tracking: refresh vs loadMore', () => {
-    it('refresh during an in-flight loadMore runs its reset path and is not dropped', async () => {
+    const deferredPage = () => {
+      let resolve!: (page: AssetResponse) => void
+      let reject!: (err: unknown) => void
+      const promise = new Promise<AssetResponse>((res, rej) => {
+        resolve = res
+        reject = rej
+      })
+      return { promise, resolve, reject }
+    }
+
+    /**
+     * Loads a full first page so `hasMore` is true and a loadMore is legal,
+     * then clears the mock so each test's assertions see only its own calls.
+     */
+    const setupStoreWithFirstPage = async () => {
       const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) =>
         makeAsset(`a${i}`, `f${i}.png`)
       )
@@ -1811,21 +1825,18 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
       )
       const store = useAssetsStore()
       await store.updateFlatOutputs()
-
       vi.mocked(assetService.getAssetsPageByTag).mockClear()
+      return { store, firstPageIds: firstPage.map((a) => a.id) }
+    }
 
-      let resolveLoadMore!: (page: AssetResponse) => void
-      const loadMorePromise = new Promise<AssetResponse>((res) => {
-        resolveLoadMore = res
-      })
-      let resolveRefresh!: (page: AssetResponse) => void
-      const refreshPromise = new Promise<AssetResponse>((res) => {
-        resolveRefresh = res
-      })
+    it('refresh during an in-flight loadMore runs its reset path and is not dropped', async () => {
+      const { store } = await setupStoreWithFirstPage()
+      const loadMorePage = deferredPage()
+      const refreshPage = deferredPage()
 
       vi.mocked(assetService.getAssetsPageByTag)
-        .mockReturnValueOnce(loadMorePromise)
-        .mockReturnValueOnce(refreshPromise)
+        .mockReturnValueOnce(loadMorePage.promise)
+        .mockReturnValueOnce(refreshPage.promise)
 
       const loadMoreResult = store.loadMoreFlatOutputs()
       const refreshResult = store.updateFlatOutputs()
@@ -1834,46 +1845,29 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
         2
       )
 
-      resolveRefresh(makePage([makeAsset('fresh-1', 'fresh.png')]))
-      resolveLoadMore(makePage([makeAsset('extra-1', 'extra.png')]))
+      refreshPage.resolve(makePage([makeAsset('fresh-1', 'fresh.png')]))
+      loadMorePage.resolve(makePage([makeAsset('extra-1', 'extra.png')]))
       await Promise.all([loadMoreResult, refreshResult])
 
-      expect(store.flatOutputAssets.map((a) => a.id)).toContain('fresh-1')
-      expect(store.flatOutputAssets.map((a) => a.id)).not.toContain('extra-1')
+      expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['fresh-1'])
     })
 
     it('a stale loadMore that resolves before the refresh does not corrupt seenIds', async () => {
-      const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) =>
-        makeAsset(`a${i}`, `f${i}.png`)
-      )
-      vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce(
-        makePage(firstPage, { hasMore: true })
-      )
-      const store = useAssetsStore()
-      await store.updateFlatOutputs()
-
-      vi.mocked(assetService.getAssetsPageByTag).mockClear()
-
-      let resolveLoadMore!: (page: AssetResponse) => void
-      const loadMorePromise = new Promise<AssetResponse>((res) => {
-        resolveLoadMore = res
-      })
-      let resolveRefresh!: (page: AssetResponse) => void
-      const refreshPromise = new Promise<AssetResponse>((res) => {
-        resolveRefresh = res
-      })
+      const { store } = await setupStoreWithFirstPage()
+      const loadMorePage = deferredPage()
+      const refreshPage = deferredPage()
 
       vi.mocked(assetService.getAssetsPageByTag)
-        .mockReturnValueOnce(loadMorePromise)
-        .mockReturnValueOnce(refreshPromise)
+        .mockReturnValueOnce(loadMorePage.promise)
+        .mockReturnValueOnce(refreshPage.promise)
 
       const loadMoreResult = store.loadMoreFlatOutputs()
       const refreshResult = store.updateFlatOutputs()
 
       // The stale loadMore settles first, before the refresh has replaced the
       // list. Its page must be discarded rather than folded into seenIds.
-      resolveLoadMore(makePage([makeAsset('extra-1', 'extra.png')]))
-      resolveRefresh(
+      loadMorePage.resolve(makePage([makeAsset('extra-1', 'extra.png')]))
+      refreshPage.resolve(
         makePage([makeAsset('fresh-1', 'fresh.png')], { hasMore: true })
       )
       await Promise.all([loadMoreResult, refreshResult])
@@ -1893,13 +1887,75 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
       ])
     })
 
-    it('a second concurrent refresh coalesces into the first refresh promise', async () => {
-      let resolvePage!: (page: AssetResponse) => void
-      const pagePromise = new Promise<AssetResponse>((res) => {
-        resolvePage = res
-      })
+    it('discards a stale loadMore even when the refresh reports no further pages', async () => {
+      const { store } = await setupStoreWithFirstPage()
+      const loadMorePage = deferredPage()
+      const refreshPage = deferredPage()
+
+      vi.mocked(assetService.getAssetsPageByTag)
+        .mockReturnValueOnce(loadMorePage.promise)
+        .mockReturnValueOnce(refreshPage.promise)
+
+      const loadMoreResult = store.loadMoreFlatOutputs()
+      const refreshResult = store.updateFlatOutputs()
+
+      loadMorePage.resolve(
+        makePage([makeAsset('extra-1', 'extra.png')], { hasMore: true })
+      )
+      refreshPage.resolve(
+        makePage([makeAsset('fresh-1', 'fresh.png')], { hasMore: false })
+      )
+      await Promise.all([loadMoreResult, refreshResult])
+
+      expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['fresh-1'])
+      expect(store.flatOutputHasMore).toBe(false)
+    })
+
+    it('keeps the list and the loaded-id check in step when a refresh fails mid-loadMore', async () => {
+      const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
+      try {
+        const { store, firstPageIds } = await setupStoreWithFirstPage()
+        const loadMorePage = deferredPage()
+        const refreshPage = deferredPage()
+
+        vi.mocked(assetService.getAssetsPageByTag)
+          .mockReturnValueOnce(loadMorePage.promise)
+          .mockReturnValueOnce(refreshPage.promise)
+
+        const loadMoreResult = store.loadMoreFlatOutputs()
+        const refreshResult = store.updateFlatOutputs()
+
+        const err = new Error('network down')
+        refreshPage.reject(err)
+        loadMorePage.resolve(makePage([makeAsset('extra-1', 'extra.png')]))
+        await Promise.all([loadMoreResult, refreshResult])
+
+        expect(store.flatOutputError).toBe(err)
+        expect(store.flatOutputLoading).toBe(false)
+        expect(store.flatOutputIsLoadingMore).toBe(false)
+        // The failed refresh dropped nothing; the stale loadMore was discarded.
+        expect(store.flatOutputAssets.map((a) => a.id)).toEqual(firstPageIds)
+
+        // A retry restarts from the head. The re-served first page is already
+        // in seenIds, so it must not be appended a second time.
+        vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce(
+          makePage(
+            firstPageIds.map((id, i) => makeAsset(id, `f${i}.png`)),
+            { hasMore: true }
+          )
+        )
+        await store.loadMoreFlatOutputs()
+
+        expect(store.flatOutputAssets.map((a) => a.id)).toEqual(firstPageIds)
+      } finally {
+        consoleSpy.mockRestore()
+      }
+    })
+
+    it('concurrent refreshes deduplicate the network call and produce consistent state', async () => {
+      const page = deferredPage()
       vi.mocked(assetService.getAssetsPageByTag).mockReturnValueOnce(
-        pagePromise
+        page.promise
       )
 
       const store = useAssetsStore()
@@ -1910,7 +1966,7 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => {
         1
       )
 
-      resolvePage(makePage([makeAsset('only-1', 'only.png')]))
+      page.resolve(makePage([makeAsset('only-1', 'only.png')]))
       await Promise.all([r1, r2])
 
       expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['only-1'])

src/stores/assetsStore.ts

@@ -1,7 +1,7 @@
 import { useAsyncState, whenever } from '@vueuse/core'
 import { difference } from 'es-toolkit'
 import { defineStore } from 'pinia'
-import { computed, reactive, ref, shallowReactive } from 'vue'
+import { computed, reactive, ref, shallowReactive, shallowRef } from 'vue'
 import {
   mapInputFileToAssetItem,
   mapTaskOutputToAssetItem
@@ -261,97 +261,107 @@ export const useAssetsStore = defineStore('assets', () => {
   }
 
   const flatOutputAssets = ref<AssetItem[]>([])
-  const flatOutputLoading = ref(false)
   const flatOutputError = ref<unknown>(null)
   const flatOutputOffset = ref(0)
   const flatOutputHasMore = ref(true)
-  const flatOutputIsLoadingMore = ref(false)
   const flatOutputSeenIds = new Set<string>()
   let flatOutputNextCursor: string | undefined
-  let flatOutputRefreshInFlight: Promise<AssetItem[]> | null = null
-  let flatOutputLoadMoreInFlight: Promise<AssetItem[]> | null = null
+  const flatOutputRefreshInFlight = shallowRef<Promise<AssetItem[]> | null>(
+    null
+  )
+  const flatOutputLoadMoreInFlight = shallowRef<Promise<void> | null>(null)
+  const flatOutputLoading = computed(
+    () => flatOutputRefreshInFlight.value !== null
+  )
+  const flatOutputIsLoadingMore = computed(
+    () => flatOutputLoadMoreInFlight.value !== null
+  )
   // Incremented on each refresh; loadMore results captured from a prior epoch
   // are discarded so a stale page can't append onto a freshly-refreshed list.
   let flatOutputRefreshEpoch = 0
 
-  async function fetchFlatOutputs(loadMore: boolean): Promise<AssetItem[]> {
-    if (loadMore) {
-      if (!flatOutputHasMore.value) return flatOutputAssets.value
-      if (flatOutputLoadMoreInFlight) return flatOutputLoadMoreInFlight
-      flatOutputIsLoadingMore.value = true
-    } else {
-      if (flatOutputRefreshInFlight) return flatOutputRefreshInFlight
-      flatOutputRefreshEpoch++
-      flatOutputLoading.value = true
-      flatOutputOffset.value = 0
-      flatOutputNextCursor = undefined
-      flatOutputHasMore.value = true
-      flatOutputSeenIds.clear()
+  async function updateFlatOutputs(): Promise<AssetItem[]> {
+    if (flatOutputRefreshInFlight.value) return flatOutputRefreshInFlight.value
+
+    flatOutputRefreshEpoch++
+    flatOutputOffset.value = 0
+    flatOutputNextCursor = undefined
+    flatOutputHasMore.value = true
+    flatOutputError.value = null
+
+    const inFlight = (async () => {
+      try {
+        const page = await assetService.getAssetsPageByTag(OUTPUT_TAG, true, {
+          limit: FLAT_OUTPUT_PAGE_SIZE,
+          offset: 0
+        })
+        // Swapped only on success, so a failed refresh leaves the loaded-id
+        // check consistent with the list still on screen.
+        flatOutputSeenIds.clear()
+        for (const asset of page.assets) flatOutputSeenIds.add(asset.id)
+        flatOutputAssets.value = page.assets
+        flatOutputOffset.value = page.assets.length
+        flatOutputNextCursor = page.next_cursor || undefined
+        flatOutputHasMore.value = page.assets.length > 0 && page.has_more
+        return flatOutputAssets.value
+      } catch (err) {
+        flatOutputError.value = err
+        console.error('Failed to fetch output assets:', err)
+        return []
+      } finally {
+        flatOutputRefreshInFlight.value = null
+      }
+    })()
+
+    // Assigned after the IIFE starts but before it can settle, so the `finally`
+    // above never clears a slot that has not been filled yet.
+    flatOutputRefreshInFlight.value = inFlight
+    return inFlight
+  }
+
+  async function loadMoreFlatOutputs(): Promise<void> {
+    if (!flatOutputHasMore.value) return
+    if (flatOutputLoadMoreInFlight.value) {
+      await flatOutputLoadMoreInFlight.value
+      return
     }
+
     flatOutputError.value = null
 
-    const capturedEpoch = flatOutputRefreshEpoch
+    const capturedRefreshEpoch = flatOutputRefreshEpoch
+    const requestedAfter = flatOutputNextCursor
 
     const inFlight = (async () => {
-      const requestedAfter = loadMore ? flatOutputNextCursor : undefined
       try {
         const page = await assetService.getAssetsPageByTag(OUTPUT_TAG, true, {
           limit: FLAT_OUTPUT_PAGE_SIZE,
           ...(requestedAfter
             ? { after: requestedAfter }
             : { offset: flatOutputOffset.value })
         })
-        if (loadMore && capturedEpoch !== flatOutputRefreshEpoch) {
-          return flatOutputAssets.value
-        }
+        if (capturedRefreshEpoch !== flatOutputRefreshEpoch) return
+
         const batch = page.assets
-        const fresh = loadMore
-          ? batch.filter((asset) => !flatOutputSeenIds.has(asset.id))
-          : batch
+        const fresh = batch.filter((asset) => !flatOutputSeenIds.has(asset.id))
         for (const asset of fresh) flatOutputSeenIds.add(asset.id)
-        flatOutputAssets.value = loadMore
-          ? [...flatOutputAssets.value, ...fresh]
-          : batch
+        flatOutputAssets.value = [...flatOutputAssets.value, ...fresh]
         flatOutputOffset.value += batch.length
         const nextCursor = page.next_cursor || undefined
         const cursorStuck =
           nextCursor !== undefined && nextCursor === requestedAfter
         flatOutputNextCursor = cursorStuck ? undefined : nextCursor
         flatOutputHasMore.value =
           fresh.length > 0 && page.has_more && !cursorStuck
-        return flatOutputAssets.value
       } catch (err) {
         flatOutputError.value = err
         console.error('Failed to fetch output assets:', err)
-        return loadMore ? flatOutputAssets.value : []
       } finally {
-        if (loadMore) {
-          flatOutputIsLoadingMore.value = false
-          flatOutputLoadMoreInFlight = null
-        } else {
-          flatOutputLoading.value = false
-          flatOutputRefreshInFlight = null
-        }
+        flatOutputLoadMoreInFlight.value = null
       }
     })()
 
-    // The in-flight promise is assigned synchronously here, before any await
-    // in inFlight can settle, so the loading flag set above and the guard below
-    // stay in lockstep. This relies on single-entry via updateFlatOutputs /
-    // loadMoreFlatOutputs; do not call fetchFlatOutputs directly.
-    if (loadMore) {
-      flatOutputLoadMoreInFlight = inFlight
-    } else {
-      flatOutputRefreshInFlight = inFlight
-    }
-
-    return inFlight
-  }
-
-  const updateFlatOutputs = () => fetchFlatOutputs(false)
-  const loadMoreFlatOutputs = async () => {
-    if (flatOutputIsLoadingMore.value) return
-    await fetchFlatOutputs(true)
+    flatOutputLoadMoreInFlight.value = inFlight
+    await inFlight
   }
 
   /**