Skip to content
Merged
70 changes: 70 additions & 0 deletions src/composables/useFeatureFlags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ServerFeatureFlag,
useFeatureFlags
} from '@/composables/useFeatureFlags'
import { httpSupportsModelTypeTags } from '@/platform/assets/composables/useModelTypeTagsRefresh'
import * as distributionTypes from '@/platform/distribution/types'
import {
cachedBillingControlEnabled,
Expand Down Expand Up @@ -383,6 +384,75 @@ describe('useFeatureFlags', () => {
})
})

describe('supportsModelTypeTags', () => {
afterEach(() => {
httpSupportsModelTypeTags.value = undefined
})

it('prefers the HTTP /features value over the websocket flag', () => {
vi.mocked(api.getServerFeature).mockReturnValue(true)
httpSupportsModelTypeTags.value = false

const { flags } = useFeatureFlags()

expect(flags.supportsModelTypeTags).toBe(false)
expect(api.getServerFeature).not.toHaveBeenCalled()
})

it('uses an HTTP-served true value', () => {
vi.mocked(api.getServerFeature).mockReturnValue(false)
httpSupportsModelTypeTags.value = true

const { flags } = useFeatureFlags()

expect(flags.supportsModelTypeTags).toBe(true)
})

it('falls back to the websocket flag when HTTP has not served the key', () => {
vi.mocked(api.getServerFeature).mockImplementation(
(path, defaultValue) => {
if (path === ServerFeatureFlag.SUPPORTS_MODEL_TYPE_TAGS) return true
return defaultValue
}
)

const { flags } = useFeatureFlags()

expect(flags.supportsModelTypeTags).toBe(true)
expect(api.getServerFeature).toHaveBeenCalledWith(
ServerFeatureFlag.SUPPORTS_MODEL_TYPE_TAGS,
false
)
})

it('defaults to false when neither source has the flag', () => {
vi.mocked(api.getServerFeature).mockImplementation(
(_path, defaultValue) => defaultValue
)

const { flags } = useFeatureFlags()

expect(flags.supportsModelTypeTags).toBe(false)
})

it('leaves other server flags on the websocket path', () => {
httpSupportsModelTypeTags.value = false
vi.mocked(api.getServerFeature).mockImplementation(
(path, defaultValue) => {
if (path === ServerFeatureFlag.SUPPORTS_PREVIEW_METADATA) return true
return defaultValue
}
)

const { flags } = useFeatureFlags()

expect(flags.supportsPreviewMetadata).toBe(true)
expect(api.getServerFeature).toHaveBeenCalledWith(
ServerFeatureFlag.SUPPORTS_PREVIEW_METADATA
)
})
})

describe('unifiedCloudAuthEnabled', () => {
afterEach(() => {
localStorage.clear()
Expand Down
16 changes: 13 additions & 3 deletions src/composables/useFeatureFlags.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { computed, reactive, readonly } from 'vue'
import type { Ref } from 'vue'

import { httpSupportsModelTypeTags } from '@/platform/assets/composables/useModelTypeTagsRefresh'
import { isCloud, isNightly } from '@/platform/distribution/types'
import {
cachedBillingControlEnabled,
Expand Down Expand Up @@ -238,10 +239,19 @@ export function useFeatureFlags() {
'off'
)
},
/**
* Server capability, deliberately not resolved through remoteConfig:
* refreshRemoteConfig clears to {} on any fetch error (would flap this
* flag and trigger spurious model reloads), lacks reconnect/visibility
* triggers, and has no response-ordering guard (FE-1439 tracks
* consolidation). The HTTP `/features` value is preferred because it can
* refresh mid-session; the websocket copy only arrives once per
* connection and covers backends without the HTTP key.
*/
get supportsModelTypeTags() {
return api.getServerFeature(
ServerFeatureFlag.SUPPORTS_MODEL_TYPE_TAGS,
false
return (
httpSupportsModelTypeTags.value ??

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why different pattern for this flag?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag resolves differently because it's a server capability that must be able to change mid-session: the WS copy (api.getServerFeature) arrives once per connection and is never re-pushed, and the remoteConfig-based resolution used by the other flags inherits refreshRemoteConfig's problems — it clears remoteConfig to {} on any fetch error (which would flap this flag and cause spurious model-library reloads), has no reconnect/visibility triggers, and has no out-of-order response guard. So this one prefers a dedicated refreshable HTTP value and falls back to the WS copy for backends that don't serve the key over HTTP.

To be clear, the earlier code comment here claiming a dynamic-config entry could "shadow" the capability was wrong (remoteConfig is the verbatim /features response) and has been rewritten at head 837eaaa with the real rationale above. The duplication is acknowledged as temporary: hardening refreshRemoteConfig and folding the dedicated poller into it is tracked in FE-1439 (assigned to Simon), after which this flag can resolve through the common path.


Generated by Claude Code

Comment thread
claude[bot] marked this conversation as resolved.
Outdated
api.getServerFeature(ServerFeatureFlag.SUPPORTS_MODEL_TYPE_TAGS, false)
)
}
})
Expand Down
188 changes: 188 additions & 0 deletions src/platform/assets/composables/useModelTypeTagsRefresh.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { effectScope } from 'vue'
import type { EffectScope } from 'vue'

import {
httpSupportsModelTypeTags,
refreshSupportsModelTypeTags,
useSupportsModelTypeTagsRefresh
} from '@/platform/assets/composables/useModelTypeTagsRefresh'
import { api } from '@/scripts/api'

function buildResponse(
body: unknown,
init: { ok?: boolean; status?: number } = {}
): Response {
return {
ok: init.ok ?? true,
status: init.status ?? 200,
json: vi.fn().mockResolvedValue(body)
} as unknown as Response
}

const fetchApiSpy = vi.spyOn(api, 'fetchApi')

beforeEach(() => {
vi.resetAllMocks()
httpSupportsModelTypeTags.value = undefined
})

describe('refreshSupportsModelTypeTags', () => {
it('reads the flag from the raw /features response', async () => {
fetchApiSpy.mockResolvedValue(
buildResponse({ supports_model_type_tags: true })
)

await refreshSupportsModelTypeTags()

expect(fetchApiSpy).toHaveBeenCalledWith('/features', {
cache: 'no-store'
})
expect(httpSupportsModelTypeTags.value).toBe(true)
})

it('tracks a flip to false', async () => {
httpSupportsModelTypeTags.value = true
fetchApiSpy.mockResolvedValue(
buildResponse({ supports_model_type_tags: false })
)

await refreshSupportsModelTypeTags()

expect(httpSupportsModelTypeTags.value).toBe(false)
})

it('clears the value when the backend stops serving the key', async () => {
httpSupportsModelTypeTags.value = true
fetchApiSpy.mockResolvedValue(buildResponse({ other_flag: true }))

await refreshSupportsModelTypeTags()

expect(httpSupportsModelTypeTags.value).toBeUndefined()
})

it('ignores a non-boolean value', async () => {
fetchApiSpy.mockResolvedValue(
buildResponse({ supports_model_type_tags: 'yes' })
)

await refreshSupportsModelTypeTags()

expect(httpSupportsModelTypeTags.value).toBeUndefined()
})

it('keeps the last known value on an error response', async () => {
httpSupportsModelTypeTags.value = true
fetchApiSpy.mockResolvedValue(buildResponse({}, { ok: false, status: 500 }))

await refreshSupportsModelTypeTags()

expect(httpSupportsModelTypeTags.value).toBe(true)
})

it('keeps the last known value when the fetch fails', async () => {
httpSupportsModelTypeTags.value = false
fetchApiSpy.mockRejectedValue(new Error('offline'))

await refreshSupportsModelTypeTags()

expect(httpSupportsModelTypeTags.value).toBe(false)
})

it('discards a superseded response that resolves after a newer one', async () => {
let resolveSlow: (response: Response) => void
fetchApiSpy.mockReturnValueOnce(
new Promise<Response>((resolve) => {
resolveSlow = resolve
})
)
const slowRefresh = refreshSupportsModelTypeTags()

fetchApiSpy.mockResolvedValueOnce(
buildResponse({ supports_model_type_tags: false })
)
await refreshSupportsModelTypeTags()
expect(httpSupportsModelTypeTags.value).toBe(false)

resolveSlow!(buildResponse({ supports_model_type_tags: true }))
await slowRefresh

expect(httpSupportsModelTypeTags.value).toBe(false)
})
})

describe('useSupportsModelTypeTagsRefresh', () => {
let scope: EffectScope

beforeEach(() => {
fetchApiSpy.mockResolvedValue(
buildResponse({ supports_model_type_tags: true })
)
scope = effectScope()
})

afterEach(() => {
scope.stop()
vi.useRealTimers()
})

it('fetches immediately and again on websocket reconnect', async () => {
scope.run(() => useSupportsModelTypeTagsRefresh())
expect(fetchApiSpy).toHaveBeenCalledTimes(1)

api.dispatchCustomEvent('reconnected')

expect(fetchApiSpy).toHaveBeenCalledTimes(2)
await vi.waitFor(() => expect(httpSupportsModelTypeTags.value).toBe(true))
})

it('re-fetches when the tab becomes visible again', () => {
scope.run(() => useSupportsModelTypeTagsRefresh())
expect(fetchApiSpy).toHaveBeenCalledTimes(1)

document.dispatchEvent(new Event('visibilitychange'))

expect(fetchApiSpy).toHaveBeenCalledTimes(2)
})

it('re-fetches on the polling interval', async () => {
vi.useFakeTimers()
scope.run(() => useSupportsModelTypeTagsRefresh())
expect(fetchApiSpy).toHaveBeenCalledTimes(1)

await vi.advanceTimersByTimeAsync(120_000)
expect(fetchApiSpy).toHaveBeenCalledTimes(2)
await vi.advanceTimersByTimeAsync(120_000)
expect(fetchApiSpy).toHaveBeenCalledTimes(3)
})

it('does not fetch on visibility events or interval ticks while hidden', async () => {
vi.useFakeTimers()
const hiddenSpy = vi.spyOn(document, 'hidden', 'get').mockReturnValue(false)
scope.run(() => useSupportsModelTypeTagsRefresh())
expect(fetchApiSpy).toHaveBeenCalledTimes(1)

hiddenSpy.mockReturnValue(true)
document.dispatchEvent(new Event('visibilitychange'))
await vi.advanceTimersByTimeAsync(120_000)
expect(fetchApiSpy).toHaveBeenCalledTimes(1)

hiddenSpy.mockReturnValue(false)
document.dispatchEvent(new Event('visibilitychange'))
expect(fetchApiSpy).toHaveBeenCalledTimes(2)
hiddenSpy.mockRestore()
})

it('stops all refresh triggers when the scope is disposed', async () => {
vi.useFakeTimers()
scope.run(() => useSupportsModelTypeTagsRefresh())
expect(fetchApiSpy).toHaveBeenCalledTimes(1)

scope.stop()
api.dispatchCustomEvent('reconnected')
document.dispatchEvent(new Event('visibilitychange'))
await vi.advanceTimersByTimeAsync(120_000)

expect(fetchApiSpy).toHaveBeenCalledTimes(1)
})
})
66 changes: 66 additions & 0 deletions src/platform/assets/composables/useModelTypeTagsRefresh.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { useEventListener, useIntervalFn } from '@vueuse/core'
import { ref } from 'vue'
import { z } from 'zod'

import { api } from '@/scripts/api'

const REFRESH_INTERVAL_MS = 120_000

const featuresResponseSchema = z.object({
supports_model_type_tags: z.boolean().optional()
})

/**
* `supports_model_type_tags` as served by HTTP `GET /features`, or undefined
* until a fetch has seen the key. The websocket copy of the flag is sent only
* once per connection, so an open session never observes a server-side flip
* (a rollback leaves stale tabs on the wrong tagging scheme); this HTTP
* source exists so the flag can refresh mid-session. Deliberately not
* sourced from remoteConfig: refreshRemoteConfig clears to {} on any fetch
* error (would flap this flag and trigger spurious model reloads), lacks
* reconnect/visibility triggers, and has no response-ordering guard.
* Consolidation tracked in FE-1439.
*/
export const httpSupportsModelTypeTags = ref<boolean | undefined>(undefined)

let refreshSequence = 0

export async function refreshSupportsModelTypeTags(): Promise<void> {
const sequence = ++refreshSequence
try {
const response = await api.fetchApi('/features', { cache: 'no-store' })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we doing another fetch of features here? Why can't we piggyback off of useFeatureFlags?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair question — the duplication with refreshRemoteConfig's /api/features poller is real, and the comment that originally justified this file (claiming a dynamic-config entry could "shadow" the server capability) was wrong: remoteConfig is the verbatim /features response, so nothing shadows anything. That comment has been rewritten at head 837eaaa.

The real reasons this doesn't piggyback on refreshRemoteConfig today:

  1. Clear-on-error flap: refreshRemoteConfig sets remoteConfig to {} on any fetch error, which would flip supports_model_type_tags to undefined and trigger spurious model-library reloads on transient network blips. This poller keeps the last known value on error.
  2. No reconnect/visibility triggers: it only refreshes on a 600s interval (cloud-only), so a flag flip or rollback wouldn't reach an open tab for up to 10 minutes; this feature needs refresh on WS reconnect and tab-visible.
  3. No response-ordering guard: overlapping fetches could let a stale response overwrite a newer one; this poller has a monotonic sequence guard.

The right fix is to harden refreshRemoteConfig (keep-last-value on error, ordering guard, reconnect/visibility triggers) and fold this poller into it so there's one /features poller and one source of truth. That's tracked in FE-1439, assigned to Simon.


Generated by Claude Code

Comment thread
claude[bot] marked this conversation as resolved.
Outdated
if (!response.ok) return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — Returning early on any !response.ok retains the last HTTP-served value, so once /features has served true and the endpoint later disappears (e.g. a backend downgrade returning 404), the flag is stuck true: the ?? in useFeatureFlags never falls through to the correct websocket value. Clear httpSupportsModelTypeTags on a 404 (endpoint gone) while keeping it on transient 5xx. Raised by 1 of 8 reviewers (gemini-3.1-pro edge-case).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retain-on-error is deliberate and test-pinned ("keeps the last known value on an error response" in useModelTypeTagsRefresh.test.ts): clearing on 404 would reintroduce flag flap during rolling deploys, where mixed pod sets behind a load balancer can briefly 404 the route while the websocket copy lacks the key — flipping the effective flag and firing a spurious full model-library reload, which is exactly the failure mode this composable exists to avoid. Note a 200 response without the key already clears the value (useModelTypeTagsRefresh.ts:45-47), so the common downgrade case converges correctly. The residual window is only a downgrade to a backend predating /features entirely, where the websocket value takes over on the next page load — an accepted trade-off for this PR's scope.


Generated by Claude Code

const features: unknown = await response.json()
// The refresh triggers can overlap (reconnect, visibility, interval); a
// superseded fetch must not commit, or a slow pre-flip response could
// revert a newer value.
if (sequence !== refreshSequence) return
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
const parsed = featuresResponseSchema.safeParse(features)
httpSupportsModelTypeTags.value = parsed.success
? parsed.data.supports_model_type_tags
: undefined
} catch {
// A failed fetch keeps the last known value; a backend that never serves
// the key stays undefined and the websocket flag remains authoritative.
}
}

/**
* Keeps {@link httpSupportsModelTypeTags} current for the app's lifetime:
* fetches immediately, again whenever the websocket reconnects or the tab
* returns to the foreground, and on a slow interval while visible. The server
* caches `/features` responses, so the interval is cheap.
*/
export function useSupportsModelTypeTagsRefresh(): void {
void refreshSupportsModelTypeTags()
useEventListener(api, 'reconnected', () => {
void refreshSupportsModelTypeTags()
})
useEventListener(document, 'visibilitychange', () => {
if (!document.hidden) void refreshSupportsModelTypeTags()
})
useIntervalFn(() => {
if (document.hidden) return
void refreshSupportsModelTypeTags()
}, REFRESH_INTERVAL_MS)
}
Loading
Loading