From aa46a81a0ff737fd607f5c58f19a92a1eded2335 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 5 Oct 2026 14:43:49 +0300 Subject: [PATCH 1/4] fix(web): clients/profiles stale-async and scope-list fixes (refs #1446) Shared GET /clients ticket, unscoped roster for the sidebar badge, assign dialog and profiles page, per-client preview tickets with Connect disabled while a changed intent refreshes, refusal clearing on success and cancel, BulkMove counting/failed state with per-open ticket, AgentTokens load ticket and a non-wrapping Profile chip (items 3,4,6,7,8,11,12,13,14,17). --- frontend/src/components/ClientConnectList.vue | 24 +- .../src/components/clients/BulkMoveDialog.vue | 28 ++- .../profiles/AssignClientDialog.vue | 5 +- frontend/src/stores/clients.ts | 57 ++++- frontend/src/views/AgentTokens.vue | 11 +- frontend/src/views/Profiles.vue | 2 +- .../unit/agent-tokens-stale-load-1446.spec.ts | 60 +++++ .../unit/clients-stale-async-1446.spec.ts | 236 ++++++++++++++++++ .../tests/unit/profiles-sse-refresh.spec.ts | 8 +- 9 files changed, 404 insertions(+), 27 deletions(-) create mode 100644 frontend/tests/unit/agent-tokens-stale-load-1446.spec.ts create mode 100644 frontend/tests/unit/clients-stale-async-1446.spec.ts diff --git a/frontend/src/components/ClientConnectList.vue b/frontend/src/components/ClientConnectList.vue index 0a2e756e3..3bf58ffcf 100644 --- a/frontend/src/components/ClientConnectList.vue +++ b/frontend/src/components/ClientConnectList.vue @@ -281,7 +281,7 @@ :data-test="`client-preview-confirm-${client.id}`" @click="confirmConnect(client.id)" class="btn btn-primary btn-xs" - :disabled="loading.clients[client.id] || previews[client.id]!.access_state === 'malformed'" + :disabled="loading.clients[client.id] || previewRefreshing[client.id] || previews[client.id]!.access_state === 'malformed'" > Connect @@ -846,13 +846,28 @@ async function onBindingToggle(clientId: string) { await refreshPreview(clientId) } +// One ticket per client: a slow preview for an older intent must not land over +// the newer one. `previewRefreshing` keeps Connect disabled while the intent on +// screen and the precondition token in hand disagree. +const previewTickets: Record = {} +const previewRefreshing = ref>({}) +function setRefreshing(clientId: string, on: boolean) { + previewRefreshing.value = { ...previewRefreshing.value, [clientId]: on } +} + async function refreshPreview(clientId: string) { + const ticket = (previewTickets[clientId] = (previewTickets[clientId] ?? 0) + 1) setRefusal(clientId, null) + setRefreshing(clientId, true) try { const response = await fetchPreview(clientId, intentFor(clientId)) + if (ticket !== previewTickets[clientId]) return if (response.success && response.data) previews.value = { ...previews.value, [clientId]: response.data } } catch (err) { + if (ticket !== previewTickets[clientId]) return setRefusal(clientId, err as ApiError) + } finally { + if (ticket === previewTickets[clientId]) setRefreshing(clientId, false) } } @@ -886,6 +901,11 @@ async function startConnect(clientId: string) { // Cancel dismisses the preview WITHOUT writing anything (Spec 078 US1). function cancelPreview(clientId: string) { + // Invalidate any preview still in flight and drop its refusal (the bulk path + // never goes through here). + previewTickets[clientId] = (previewTickets[clientId] ?? 0) + 1 + setRefreshing(clientId, false) + setRefusal(clientId, null) const next = { ...previews.value } delete next[clientId] previews.value = next @@ -970,6 +990,8 @@ async function connect( resultMessage.value = response.data.message || `Connected to ${clientId}` resultSuccess.value = true resultReloadHint.value = response.data.reload_hint || '' + // A stale refusal from an earlier attempt no longer applies. + setRefusal(clientId, null) // Empty/absent backup_path on success means no prior file existed. const backupPath = response.data.backup_path || null resultBackupPath.value = backupPath diff --git a/frontend/src/components/clients/BulkMoveDialog.vue b/frontend/src/components/clients/BulkMoveDialog.vue index db754387a..f5ed1c1fd 100644 --- a/frontend/src/components/clients/BulkMoveDialog.vue +++ b/frontend/src/components/clients/BulkMoveDialog.vue @@ -24,12 +24,17 @@

{{ previewLine }}

+
+ Could not count every client. + +
-
@@ -75,8 +80,11 @@ const error = ref('') const result = ref(null) // POST /clients/bulk-assign moves every client on the from-profile instance-wide, // while props.clients is the page's ?profile=/?client= filtered list. The preview -// counts the unscoped list (props.clients until it arrives or if it fails). +// counts the unscoped list only: while it is pending or failed there is no +// count, and Move stays disabled (the filtered rows would undercount). const allClients = ref(null) +const loadFailed = ref(false) +let openTicket = 0 watch(() => props.open, open => { if (!open) return @@ -87,6 +95,7 @@ watch(() => props.open, open => { error.value = '' result.value = null allClients.value = null + loadFailed.value = false void loadAllClients() if (!profiles.loaded) void profiles.fetchProfiles() }) @@ -95,17 +104,24 @@ watch(() => props.open, open => { watch(to, value => { if (!value && mode.value === 'locked') mode.value = '' }) async function loadAllClients() { + // A per-load ticket: a slow response of an earlier open (or retry) is ignored. + const ticket = ++openTicket + loadFailed.value = false try { const response = await api.getClients() - if (response.success && Array.isArray(response.data?.clients) && props.open) allClients.value = response.data.clients + if (ticket !== openTicket || !props.open) return + if (response.success && Array.isArray(response.data?.clients)) allClients.value = response.data.clients + else loadFailed.value = true } catch { - // The page's rows stay the fallback. + if (ticket === openTicket) loadFailed.value = true } } -const affected = computed(() => (allClients.value ?? props.clients).filter(client => client.credential_state === 'client' && (client.profile ?? '') === from.value)) +const countPending = computed(() => allClients.value === null) +const affected = computed(() => (allClients.value ?? []).filter(client => client.credential_state === 'client' && (client.profile ?? '') === from.value)) const previewLine = computed(() => { const name = from.value ? profiles.titleFor(from.value) : 'All servers' + if (countPending.value) return loadFailed.value ? `Count unavailable for ${name}` : `Counting clients that use ${name}...` const n = affected.value.length return `${n} client${n === 1 ? ' uses' : 's use'} ${name}` }) diff --git a/frontend/src/components/profiles/AssignClientDialog.vue b/frontend/src/components/profiles/AssignClientDialog.vue index b9470a24f..1b2471caf 100644 --- a/frontend/src/components/profiles/AssignClientDialog.vue +++ b/frontend/src/components/profiles/AssignClientDialog.vue @@ -42,13 +42,14 @@ const bindings = useClientBindingsStore() const clientId = ref('') const done = ref('') const refusal = computed(() => (clientId.value ? bindings.rowErrors[clientId.value] : undefined)) -const eligible = computed(() => clients.clients.filter(client => client.credential_state === 'client')) +const eligible = computed(() => clients.allClients.filter(client => client.credential_state === 'client')) watch(() => props.open, open => { if (!open) return clientId.value = '' done.value = '' - if (!clients.clients.length) void clients.refreshPresence() + // The roster may be a scoped page's rows: always refetch the unscoped one. + void clients.refreshPresence() }) async function submit() { diff --git a/frontend/src/stores/clients.ts b/frontend/src/stores/clients.ts index a1f6ee65c..1e53da487 100644 --- a/frontend/src/stores/clients.ts +++ b/frontend/src/stores/clients.ts @@ -24,19 +24,48 @@ export const useClientsStore = defineStore('clients', () => { // A response is applied only while it is still the latest of its kind and the // scope it was asked for is still the active one: changing ?profile= / ?client= // must not let the previous scope's rows land afterwards. + // One ticket covers every GET /clients (load and refreshPresence), so an older + // response can never overwrite a newer one. loadTicket only owns the loading + // flag, so a load superseded by a presence poll still clears it. + let fetchTicket = 0 let loadTicket = 0 - let presenceTicket = 0 const scopeKey = () => JSON.stringify(scope) + const isUnscoped = () => !scope.profile && !scope.client + + // The unscoped roster (updated only by unscoped fetches). The sidebar badge, + // the assign dialog and the profile usage list read it, so a ?profile= / + // ?client= filter on the Clients page never shrinks them. + const allClients = ref([]) + // Set by clearScope(): the roster must be refetched before it is trusted. + const stale = ref(false) + // Both GET /clients reads of one refresh; the unscoped one is shared when the + // active scope is already unscoped. + async function fetchRosters() { + const scoped = api.getClients(scope) + const all = isUnscoped() ? scoped : api.getClients({}) + return Promise.all([scoped, all]) + } + function applyAll(response: { success: boolean; data?: { clients?: ClientPresence[] } }) { + if (response.success && Array.isArray(response.data?.clients)) allClients.value = response.data.clients + stale.value = false + } async function load(nextScope?: { profile?: string; client?: string }) { if (nextScope) scope = nextScope - const ticket = ++loadTicket + const ticket = ++fetchTicket + const mine = ++loadTicket const asked = scopeKey() loading.value = true error.value = null - const [clientResponse, routingResponse] = await Promise.all([api.getClients(scope), api.getRouting()]) - // A newer load owns the loading flag and the rows. - if (ticket !== loadTicket || asked !== scopeKey()) return + const [[clientResponse, allResponse], routingResponse] = await Promise.all([fetchRosters(), api.getRouting()]) + // A newer load owns the loading flag. + if (mine !== loadTicket || asked !== scopeKey()) return + // A newer fetch (another load or a presence poll) owns the rows. + if (ticket !== fetchTicket) { + loading.value = false + return + } + applyAll(allResponse) if (clientResponse.success && clientResponse.data) { clients.value = clientResponse.data.clients warnings.value = clientResponse.data.warnings ?? [] @@ -47,17 +76,18 @@ export const useClientsStore = defineStore('clients', () => { } // Rows with at least one live session: the sidebar Clients badge (Spec 109-i). - const liveCount = computed(() => clients.value.filter(client => (client.active_sessions ?? 0) > 0).length) + const liveCount = computed(() => allClients.value.filter(client => (client.active_sessions ?? 0) > 0).length) // Silent presence refresh for the sidebar badge. It fetches GET /clients only // and never touches loading/error/routing, so the Clients page does not // flash a spinner when a badge poll lands underneath it. async function refreshPresence() { - const ticket = ++presenceTicket + const ticket = ++fetchTicket const asked = scopeKey() try { - const response = await api.getClients(scope) - if (ticket !== presenceTicket || asked !== scopeKey()) return + const [response, allResponse] = await fetchRosters() + if (ticket !== fetchTicket || asked !== scopeKey()) return + applyAll(allResponse) if (response.success && Array.isArray(response.data?.clients)) { warnings.value = response.data.warnings ?? [] // GET /clients is metadata-only. For rows whose detail was loaded via @@ -97,7 +127,12 @@ export const useClientsStore = defineStore('clients', () => { else clients.value.push(row) } - function clearScope() { scope = {} } + // Dropping the scope also invalidates the rows: they were fetched under the + // old filter, so the next consumer must refetch rather than trust them. + function clearScope() { + scope = {} + stale.value = true + } async function loadDetail(id: string) { const response = await api.getClient(id) @@ -119,5 +154,5 @@ export const useClientsStore = defineStore('clients', () => { }) } - return { clients, warnings, routing, loading, error, liveCount, load, refreshPresence, loadDetail, replaceRow, clearScope } + return { clients, allClients, stale, warnings, routing, loading, error, liveCount, load, refreshPresence, loadDetail, replaceRow, clearScope } }) diff --git a/frontend/src/views/AgentTokens.vue b/frontend/src/views/AgentTokens.vue index 5ca012808..28ce42e65 100644 --- a/frontend/src/views/AgentTokens.vue +++ b/frontend/src/views/AgentTokens.vue @@ -121,7 +121,7 @@ Name Kind - Profile + Profile Mode Prefix Expires @@ -151,7 +151,7 @@ {{ isClientCredential(token) ? 'Client' : 'Agent' }} - {{ profilesStore.titleFor(token.profile_pin) }} + {{ profilesStore.titleFor(token.profile_pin) }} — @@ -558,23 +558,28 @@ function permissionBadgeClass(perm: string): string { } // Data loading +let tokensTicket = 0 async function loadTokens() { + const ticket = ++tokensTicket loading.value = true error.value = null try { const rest = scopeQuery?.toRest() const response = await apiClient.listAgentTokens({ profile: rest?.profile, token: rest?.token }) + // A newer load owns the rows and the loading flag. + if (ticket !== tokensTicket) return if (response.success && response.data) { tokens.value = response.data.tokens || [] } else { error.value = response.error || 'Failed to load tokens' } } catch (err: any) { + if (ticket !== tokensTicket) return error.value = err.message || 'Failed to load tokens' console.error('Failed to load tokens:', err) } finally { - loading.value = false + if (ticket === tokensTicket) loading.value = false } } diff --git a/frontend/src/views/Profiles.vue b/frontend/src/views/Profiles.vue index 8bde5f97b..895ae8b97 100644 --- a/frontend/src/views/Profiles.vue +++ b/frontend/src/views/Profiles.vue @@ -112,6 +112,6 @@ onMounted(() => { consumeCreateParam() void store.fetchProfiles() // Who uses a profile (client names) comes from the clients store. - if (!tenant.value && !clients.clients.length) void clients.refreshPresence() + if (!tenant.value && (!clients.allClients.length || clients.stale)) void clients.refreshPresence() }) diff --git a/frontend/tests/unit/agent-tokens-stale-load-1446.spec.ts b/frontend/tests/unit/agent-tokens-stale-load-1446.spec.ts new file mode 100644 index 000000000..fd92b1763 --- /dev/null +++ b/frontend/tests/unit/agent-tokens-stale-load-1446.spec.ts @@ -0,0 +1,60 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushPromises, mount } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import { createMemoryHistory, createRouter } from 'vue-router' +import AgentTokens from '@/views/AgentTokens.vue' +import api from '@/services/api' +import { setAvailableFeatures } from '@/composables/useScopeQuery' + +// Issue #1446 item 8: an older GET /tokens must not overwrite a newer one. +// Item 17: the Profile column keeps a minimum width and its chip never wraps. + +vi.mock('@/services/api', () => ({ + default: { + hasAPIKey: vi.fn(() => true), + listAgentTokens: vi.fn(), + getServers: vi.fn(), + getProfiles: vi.fn(), + }, +})) + +const future = '2030-01-01T00:00:00Z' +const tok = (name: string) => ({ name, token_prefix: 'mcp_agt_aa', allowed_servers: ['*'], permissions: ['read'], expires_at: future, created_at: future, last_used_at: null, revoked: false, kind: 'agent', legacy_scope: false }) +const stub = { template: '
' } + +describe('Agent tokens list (#1446)', () => { + const original = (HTMLDialogElement.prototype as any).showModal + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + ;(HTMLDialogElement.prototype as any).showModal = vi.fn() + setAvailableFeatures(['scope_filters']) + ;(api.getServers as any).mockResolvedValue({ success: true, data: { servers: [] } }) + ;(api.getProfiles as any).mockResolvedValue({ profiles: [] }) + }) + afterEach(() => { ;(HTMLDialogElement.prototype as any).showModal = original }) + + it('the latest load wins when responses resolve out of order', async () => { + let resolveFirst!: (v: any) => void + ;(api.listAgentTokens as any) + .mockReturnValueOnce(new Promise(r => { resolveFirst = r })) + .mockResolvedValue({ success: true, data: { tokens: [tok('newer')] } }) + const router = createRouter({ history: createMemoryHistory(), routes: [{ path: '/clients', name: 'clients', component: stub }, { path: '/profiles', name: 'profiles', component: stub }] }) + await router.push('/clients?tab=tokens') + await router.isReady() + const wrapper = mount(AgentTokens, { global: { plugins: [router] }, attachTo: document.body }) + await vi.waitFor(() => expect((api.listAgentTokens as any).mock.calls.length).toBeGreaterThanOrEqual(1)) + const refresh = (wrapper.vm as any).refreshTokens ?? (wrapper.vm as any).loadTokens + if (refresh) { + await refresh() + } else { + await router.push('/clients?tab=tokens&profile=x') + } + await flushPromises() + resolveFirst({ success: true, data: { tokens: [tok('older')] } }) + await flushPromises() + expect(wrapper.find('[data-test="token-row-newer"]').exists()).toBe(true) + expect(wrapper.find('[data-test="token-row-older"]').exists()).toBe(false) + wrapper.unmount() + }) +}) diff --git a/frontend/tests/unit/clients-stale-async-1446.spec.ts b/frontend/tests/unit/clients-stale-async-1446.spec.ts new file mode 100644 index 000000000..348d9bbd4 --- /dev/null +++ b/frontend/tests/unit/clients-stale-async-1446.spec.ts @@ -0,0 +1,236 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { flushPromises, mount } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import api from '@/services/api' +import { useClientsStore } from '@/stores/clients' +import { useProfilesStore } from '@/stores/profiles' +import AssignClientDialog from '@/components/profiles/AssignClientDialog.vue' +import BulkMoveDialog from '@/components/clients/BulkMoveDialog.vue' +import ClientConnectList from '@/components/ClientConnectList.vue' +import { WORK_RO, WORK_FULL, makeClient } from './fixtures/profiles108i' + +// Issue #1446 (Spec 108-i review): stale-async and scope-list follow-ups. + +vi.mock('@/services/api', () => ({ + default: { + hasAPIKey: vi.fn(() => true), + getClients: vi.fn(), + getClient: vi.fn(), + getRouting: vi.fn(), + getProfiles: vi.fn(), + setClientBinding: vi.fn(), + bulkAssignClients: vi.fn(), + getConnectStatus: vi.fn(), + getConnectClientStatus: vi.fn(), + getConnectPreview: vi.fn(), + connectClient: vi.fn(), + disconnectClient: vi.fn(), + getOnboardingState: vi.fn(), + }, +})) + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise(r => { resolve = r }) + return { promise, resolve } +} +const ok = (clients: any[]) => ({ success: true, data: { clients, warnings: [] } }) +const live = (id: string) => makeClient(id, { active_sessions: 1 }) +const idle = (id: string) => makeClient(id, { active_sessions: 0 }) + +beforeEach(() => { + setActivePinia(createPinia()) + for (const fn of Object.values(api) as any[]) fn.mockReset?.() + ;(api.hasAPIKey as any).mockReturnValue(true) + ;(api.getRouting as any).mockResolvedValue({ success: true, data: null }) +}) + +describe('clients store (1446-4, 1446-6, 1446-3)', () => { + it('an older load() cannot overwrite a newer refreshPresence()', async () => { + const store = useClientsStore() + const slow = deferred() + ;(api.getClients as any).mockReturnValueOnce(slow.promise) + const loading = store.load() + ;(api.getClients as any).mockResolvedValue(ok([live('new')])) + await store.refreshPresence() + expect(store.clients.map(c => c.id)).toEqual(['new']) + slow.resolve(ok([idle('old')])) + await loading + expect(store.clients.map(c => c.id)).toEqual(['new']) + expect(store.loading).toBe(false) + }) + + it('an older refreshPresence() cannot overwrite a newer load()', async () => { + const store = useClientsStore() + const slow = deferred() + ;(api.getClients as any).mockReturnValueOnce(slow.promise) + const polling = store.refreshPresence() + ;(api.getClients as any).mockResolvedValue(ok([live('new')])) + await store.load() + slow.resolve(ok([idle('old')])) + await polling + expect(store.clients.map(c => c.id)).toEqual(['new']) + }) + + it('liveCount counts the unscoped roster, so a scoped page load does not shrink it', async () => { + const store = useClientsStore() + ;(api.getClients as any).mockImplementation(async (scope: any = {}) => ok(scope?.client ? [live('cursor')] : [live('cursor'), live('codex'), idle('zed')])) + await store.load({ client: 'cursor' }) + expect(store.clients.map(c => c.id)).toEqual(['cursor']) + expect(store.liveCount).toBe(2) + await store.refreshPresence() + expect(store.clients).toHaveLength(1) + expect(store.liveCount).toBe(2) + }) + + it('clearScope() marks the unscoped roster for refetch until the next refresh', async () => { + const store = useClientsStore() + ;(api.getClients as any).mockResolvedValue(ok([live('cursor')])) + await store.load({ client: 'cursor' }) + store.clearScope() + expect(store.stale).toBe(true) + await store.refreshPresence() + expect(store.stale).toBe(false) + }) +}) + +describe('AssignClientDialog (1446-3)', () => { + it('lists eligible clients from the unscoped roster and refreshes it on open even when scoped rows exist', async () => { + const store = useClientsStore() + const bound = (id: string) => makeClient(id, { credential_state: 'client' }) + ;(api.getClients as any).mockImplementation(async (scope: any = {}) => ok(scope?.client ? [bound('cursor')] : [bound('cursor'), bound('codex')])) + await store.load({ client: 'cursor' }) + vi.mocked(api.getClients).mockClear() + HTMLDialogElement.prototype.showModal = vi.fn() + const wrapper = mount(AssignClientDialog, { props: { open: false, profileName: 'work-ro' } }) + await wrapper.setProps({ open: true }) + await flushPromises() + expect(api.getClients).toHaveBeenCalled() + const options = wrapper.findAll('option').map(o => o.attributes('value')) + expect(options).toContain('codex') + expect(options).toContain('cursor') + }) +}) + +describe('BulkMoveDialog (1446-13, 1446-14)', () => { + function mountDialog() { + HTMLDialogElement.prototype.showModal = vi.fn() + const profiles = useProfilesStore() + profiles.profiles = [WORK_RO, WORK_FULL] as any + return mount(BulkMoveDialog, { props: { open: false, clients: [makeClient('scoped', { profile: 'work-ro' })], initialFrom: 'work-ro' } }) + } + + it('shows counting and disables Move while the unscoped list is pending, never the filtered rows', async () => { + const wrapper = mountDialog() + const pending = deferred() + ;(api.getClients as any).mockReturnValue(pending.promise) + await wrapper.setProps({ open: true }) + await flushPromises() + expect(wrapper.get('[data-test="bulk-preview-line"]').text()).toMatch(/Counting/i) + await wrapper.get('[data-test="bulk-to"]').setValue('work-full') + expect(wrapper.get('[data-test="bulk-submit"]').attributes('disabled')).toBeDefined() + pending.resolve(ok([makeClient('a', { profile: 'work-ro' }), makeClient('b', { profile: 'work-ro' })])) + await flushPromises() + expect(wrapper.get('[data-test="bulk-preview-line"]').text()).toMatch(/^2 clients use /) + expect(wrapper.get('[data-test="bulk-submit"]').attributes('disabled')).toBeUndefined() + }) + + it('a failed load keeps Move disabled and offers a retry', async () => { + const wrapper = mountDialog() + ;(api.getClients as any).mockRejectedValueOnce(new Error('offline')) + await wrapper.setProps({ open: true }) + await flushPromises() + await wrapper.get('[data-test="bulk-to"]').setValue('work-full') + expect(wrapper.find('[data-test="bulk-count-failed"]').exists()).toBe(true) + expect(wrapper.get('[data-test="bulk-submit"]').attributes('disabled')).toBeDefined() + ;(api.getClients as any).mockResolvedValue(ok([makeClient('a', { profile: 'work-ro' })])) + await wrapper.get('[data-test="bulk-count-retry"]').trigger('click') + await flushPromises() + expect(wrapper.find('[data-test="bulk-count-failed"]').exists()).toBe(false) + expect(wrapper.get('[data-test="bulk-submit"]').attributes('disabled')).toBeUndefined() + }) + + it('a slow response of an earlier open is ignored', async () => { + const wrapper = mountDialog() + const first = deferred() + ;(api.getClients as any).mockReturnValueOnce(first.promise) + await wrapper.setProps({ open: true }) + await wrapper.setProps({ open: false }) + ;(api.getClients as any).mockResolvedValueOnce(ok([makeClient('fresh', { profile: 'work-ro' })])) + await wrapper.setProps({ open: true }) + await flushPromises() + first.resolve(ok([makeClient('x', { profile: 'work-ro' }), makeClient('y', { profile: 'work-ro' }), makeClient('z', { profile: 'work-ro' })])) + await flushPromises() + expect(wrapper.get('[data-test="bulk-preview-line"]').text()).toMatch(/^1 client uses /) + }) +}) + +describe('ClientConnectList (1446-7, 1446-11, 1446-12)', () => { + const guard = Object.assign(new Error('refused'), { name: 'ApiError', error: 'GUARD-TEXT', code: 'binding_bypassable_without_auth', status: 409, fixes: [{ kind: 'require_mcp_auth' }] }) + function previewOf(token: string) { + return { + success: true, + data: { client: 'cursor', config_path: '/x/mcp.json', display_path: '~/x/mcp.json', format: 'json', server_key: 'mcpServers', server_name: 'mcpproxy', entry_text: '{}', entry_exists: false, contains_api_key: false, access_state: 'accessible', precondition_token: token }, + } + } + async function open() { + ;(api.getConnectStatus as any).mockResolvedValue({ success: true, data: [{ id: 'cursor', name: 'Cursor', config_path: '/x/mcp.json', exists: true, connected: false, supported: true, icon: 'cursor' }] }) + ;(api.getOnboardingState as any).mockResolvedValue({ success: true, data: null }) + ;(api.getClients as any).mockResolvedValue(ok([makeClient('cursor')])) + useProfilesStore().profiles = [WORK_RO] as any + HTMLDialogElement.prototype.showModal = vi.fn() + const wrapper = mount(ClientConnectList, { props: { show: false } }) + await wrapper.setProps({ show: true }) + await flushPromises() + return wrapper + } + + it('disables Connect while a changed intent refreshes the preview, and ignores a slower older preview', async () => { + ;(api.getConnectPreview as any).mockResolvedValueOnce(previewOf('tok-1')) + const wrapper = await open() + await wrapper.get('[data-test="connect-cursor"]').trigger('click') + await flushPromises() + const confirm = () => wrapper.get('[data-test="client-preview-confirm-cursor"]') + expect(confirm().attributes('disabled')).toBeUndefined() + + const slow = deferred() + ;(api.getConnectPreview as any).mockReturnValueOnce(slow.promise) + await wrapper.get('[data-test="connect-profile-select-cursor"]').setValue('work-ro') + await flushPromises() + expect(confirm().attributes('disabled')).toBeDefined() + + const fast = deferred() + ;(api.getConnectPreview as any).mockReturnValueOnce(fast.promise) + await wrapper.get('[data-test="connect-profile-select-cursor"]').setValue('') + fast.resolve(previewOf('tok-new')) + await flushPromises() + slow.resolve(previewOf('tok-old')) + await flushPromises() + expect(confirm().attributes('disabled')).toBeUndefined() + ;(api.connectClient as any).mockResolvedValue({ success: true, data: { success: true, message: 'ok', config_path: '/x', backup_path: '' } }) + await confirm().trigger('click') + await flushPromises() + expect((api.connectClient as any).mock.calls[0][3].precondition_token).toBe('tok-new') + }) + + it('Cancel clears a guard refusal of that client', async () => { + ;(api.getConnectPreview as any).mockResolvedValueOnce(previewOf('tok-1')) + const wrapper = await open() + await wrapper.get('[data-test="connect-cursor"]').trigger('click') + await flushPromises() + ;(api.connectClient as any).mockRejectedValue(guard) + await wrapper.get('[data-test="client-preview-confirm-cursor"]').trigger('click') + await flushPromises() + const vm: any = wrapper.vm + expect(wrapper.find('[data-test="guard-refusal"]').exists()).toBe(true) + await wrapper.get('[data-test="client-preview-cancel-cursor"]').trigger('click') + await flushPromises() + expect(vm).toBeTruthy() + expect(wrapper.find('[data-test="connect-bulk-refusals"]').exists()).toBe(false) + // Re-open: no stale refusal is rendered next to the new preview. + ;(api.getConnectPreview as any).mockResolvedValueOnce(previewOf('tok-2')) + await wrapper.get('[data-test="connect-cursor"]').trigger('click') + await flushPromises() + expect(wrapper.find('[data-test="guard-refusal"]').exists()).toBe(false) + }) +}) diff --git a/frontend/tests/unit/profiles-sse-refresh.spec.ts b/frontend/tests/unit/profiles-sse-refresh.spec.ts index 0baeeab85..43bc22cbe 100644 --- a/frontend/tests/unit/profiles-sse-refresh.spec.ts +++ b/frontend/tests/unit/profiles-sse-refresh.spec.ts @@ -55,12 +55,14 @@ describe('profiles and client rows refresh on SSE events (Spec 108-i T102)', () it('refetches the client rows on client.binding_changed, keeping the scope and the warnings', async () => { const store = useClientsStore() await store.load({ profile: 'work' }) - expect(api.getClients).toHaveBeenLastCalledWith({ profile: 'work' }) + // A scoped load also reads the unscoped roster (sidebar badge, assign dialog). + expect(api.getClients).toHaveBeenNthCalledWith(1, { profile: 'work' }) + expect(api.getClients).toHaveBeenNthCalledWith(2, {}) ;(api.getClients as any).mockResolvedValue({ success: true, data: { clients: [makeClient('cursor', { profile: 'work' })], warnings: [{ code: 'client_rotation_pending', severity: 'info', message: 'pending' }] } }) window.dispatchEvent(new CustomEvent('mcpproxy:client.binding_changed', { detail: { client_id: 'cursor' } })) await vi.advanceTimersByTimeAsync(0) - expect(api.getClients).toHaveBeenCalledTimes(2) - expect(api.getClients).toHaveBeenLastCalledWith({ profile: 'work' }) + expect(api.getClients).toHaveBeenCalledTimes(4) + expect(api.getClients).toHaveBeenNthCalledWith(3, { profile: 'work' }) expect(store.clients[0].profile).toBe('work') expect(store.warnings.map(warning => warning.code)).toEqual(['client_rotation_pending']) }) From 4e1b521e184bad78cf38a793e751a7fb0219d119 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 5 Oct 2026 14:55:24 +0300 Subject: [PATCH 2/4] fix(web): superseded load keeps routing, stale-clear only on success, preview refresh failure and reopen guards (refs #1446) --- frontend/src/components/ClientConnectList.vue | 26 ++++++++- frontend/src/stores/clients.ts | 7 ++- .../unit/clients-stale-async-1446.spec.ts | 57 +++++++++++++++++++ 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/ClientConnectList.vue b/frontend/src/components/ClientConnectList.vue index 3bf58ffcf..cdb91e2c9 100644 --- a/frontend/src/components/ClientConnectList.vue +++ b/frontend/src/components/ClientConnectList.vue @@ -281,7 +281,7 @@ :data-test="`client-preview-confirm-${client.id}`" @click="confirmConnect(client.id)" class="btn btn-primary btn-xs" - :disabled="loading.clients[client.id] || previewRefreshing[client.id] || previews[client.id]!.access_state === 'malformed'" + :disabled="loading.clients[client.id] || previewRefreshing[client.id] || previewStale[client.id] || previews[client.id]!.access_state === 'malformed'" > Connect @@ -851,6 +851,12 @@ async function onBindingToggle(clientId: string) { // screen and the precondition token in hand disagree. const previewTickets: Record = {} const previewRefreshing = ref>({}) +// A re-fetch after a binding change failed: the token in hand belongs to an +// older intent, so Connect stays disabled until a fetch succeeds or Cancel. +const previewStale = ref>({}) +function setStale(clientId: string, on: boolean) { + previewStale.value = { ...previewStale.value, [clientId]: on } +} function setRefreshing(clientId: string, on: boolean) { previewRefreshing.value = { ...previewRefreshing.value, [clientId]: on } } @@ -862,9 +868,17 @@ async function refreshPreview(clientId: string) { try { const response = await fetchPreview(clientId, intentFor(clientId)) if (ticket !== previewTickets[clientId]) return - if (response.success && response.data) previews.value = { ...previews.value, [clientId]: response.data } + if (response.success && response.data) { + previews.value = { ...previews.value, [clientId]: response.data } + setStale(clientId, false) + previewError.value = { ...previewError.value, [clientId]: '' } + } else { + setStale(clientId, true) + previewError.value = { ...previewError.value, [clientId]: response.error || 'Failed to refresh preview' } + } } catch (err) { if (ticket !== previewTickets[clientId]) return + setStale(clientId, true) setRefusal(clientId, err as ApiError) } finally { if (ticket === previewTickets[clientId]) setRefreshing(clientId, false) @@ -878,6 +892,9 @@ async function startConnect(clientId: string) { previewLoading[clientId] = true previewError.value = { ...previewError.value, [clientId]: '' } setRefusal(clientId, null) + previewTickets[clientId] = (previewTickets[clientId] ?? 0) + 1 + setRefreshing(clientId, false) + setStale(clientId, false) // The row's current binding lives in the clients store; make sure it is there. if (!clientsStore.clients.some(c => c.id === clientId)) await clientsStore.refreshPresence() delete forms[clientId] @@ -905,6 +922,7 @@ function cancelPreview(clientId: string) { // never goes through here). previewTickets[clientId] = (previewTickets[clientId] ?? 0) + 1 setRefreshing(clientId, false) + setStale(clientId, false) setRefusal(clientId, null) const next = { ...previews.value } delete next[clientId] @@ -1329,6 +1347,10 @@ watch(() => props.show, (newVal) => { previews.value = {} previewError.value = {} connectRefusal.value = {} + // Invalidate any preview fetch still in flight from the previous open. + for (const id of Object.keys(previewTickets)) previewTickets[id] += 1 + previewRefreshing.value = {} + previewStale.value = {} for (const id of Object.keys(forms)) delete forms[id] Object.assign(bulkForm, { profile: '', locked: false, touched: false }) lastConnect.value = null diff --git a/frontend/src/stores/clients.ts b/frontend/src/stores/clients.ts index 1e53da487..119c5c8ab 100644 --- a/frontend/src/stores/clients.ts +++ b/frontend/src/stores/clients.ts @@ -46,9 +46,11 @@ export const useClientsStore = defineStore('clients', () => { return Promise.all([scoped, all]) } function applyAll(response: { success: boolean; data?: { clients?: ClientPresence[] } }) { - if (response.success && Array.isArray(response.data?.clients)) allClients.value = response.data.clients + if (response.success && Array.isArray(response.data?.clients)) { + allClients.value = response.data.clients stale.value = false } + } async function load(nextScope?: { profile?: string; client?: string }) { if (nextScope) scope = nextScope @@ -60,6 +62,8 @@ export const useClientsStore = defineStore('clients', () => { const [[clientResponse, allResponse], routingResponse] = await Promise.all([fetchRosters(), api.getRouting()]) // A newer load owns the loading flag. if (mine !== loadTicket || asked !== scopeKey()) return + // Routing is only fetched here, so a superseded load still applies it. + if (routingResponse.success && routingResponse.data) routing.value = routingResponse.data // A newer fetch (another load or a presence poll) owns the rows. if (ticket !== fetchTicket) { loading.value = false @@ -71,7 +75,6 @@ export const useClientsStore = defineStore('clients', () => { warnings.value = clientResponse.data.warnings ?? [] detailLoaded.clear() } else error.value = clientResponse.error || 'Unable to load clients' - if (routingResponse.success && routingResponse.data) routing.value = routingResponse.data loading.value = false } diff --git a/frontend/tests/unit/clients-stale-async-1446.spec.ts b/frontend/tests/unit/clients-stale-async-1446.spec.ts index 348d9bbd4..bc1fd634d 100644 --- a/frontend/tests/unit/clients-stale-async-1446.spec.ts +++ b/frontend/tests/unit/clients-stale-async-1446.spec.ts @@ -60,6 +60,30 @@ describe('clients store (1446-4, 1446-6, 1446-3)', () => { expect(store.loading).toBe(false) }) + it('a load() superseded by refreshPresence() still applies its routing', async () => { + const store = useClientsStore() + const slow = deferred() + ;(api.getClients as any).mockReturnValueOnce(slow.promise) + ;(api.getRouting as any).mockResolvedValue({ success: true, data: { marker: 'routing' } }) + const loading = store.load() + ;(api.getClients as any).mockResolvedValue(ok([live('new')])) + await store.refreshPresence() + slow.resolve(ok([idle('old')])) + await loading + expect((store.routing as any)?.marker).toBe('routing') + }) + + it('a failed unscoped fetch does not clear the stale flag', async () => { + const store = useClientsStore() + ;(api.getClients as any).mockResolvedValue(ok([live('a')])) + await store.load({ profile: 'work-ro' }) + store.clearScope() + expect(store.stale).toBe(true) + ;(api.getClients as any).mockResolvedValue({ success: false, error: 'boom' }) + await store.refreshPresence() + expect(store.stale).toBe(true) + }) + it('an older refreshPresence() cannot overwrite a newer load()', async () => { const store = useClientsStore() const slow = deferred() @@ -233,4 +257,37 @@ describe('ClientConnectList (1446-7, 1446-11, 1446-12)', () => { await flushPromises() expect(wrapper.find('[data-test="guard-refusal"]').exists()).toBe(false) }) + + it('keeps Connect disabled when the preview re-fetch fails', async () => { + ;(api.getConnectPreview as any).mockResolvedValueOnce(previewOf('tok-1')) + const wrapper = await open() + await wrapper.get('[data-test="connect-cursor"]').trigger('click') + await flushPromises() + const confirm = () => wrapper.get('[data-test="client-preview-confirm-cursor"]') + ;(api.getConnectPreview as any).mockResolvedValueOnce({ success: false, error: 'nope' }) + await wrapper.get('[data-test="connect-profile-select-cursor"]').setValue('work-ro') + await flushPromises() + expect(confirm().attributes('disabled')).toBeDefined() + ;(api.getConnectPreview as any).mockRejectedValueOnce(new Error('net')) + await wrapper.get('[data-test="connect-profile-select-cursor"]').setValue('') + await flushPromises() + expect(confirm().attributes('disabled')).toBeDefined() + }) + + it('an in-flight refresh does not disable Connect after close and reopen', async () => { + ;(api.getConnectPreview as any).mockResolvedValueOnce(previewOf('tok-1')) + const wrapper = await open() + await wrapper.get('[data-test="connect-cursor"]').trigger('click') + await flushPromises() + const slow = deferred() + ;(api.getConnectPreview as any).mockReturnValueOnce(slow.promise) + await wrapper.get('[data-test="connect-profile-select-cursor"]').setValue('work-ro') + await wrapper.setProps({ show: false }) + await wrapper.setProps({ show: true }) + await flushPromises() + ;(api.getConnectPreview as any).mockResolvedValueOnce(previewOf('tok-2')) + await wrapper.get('[data-test="connect-cursor"]').trigger('click') + await flushPromises() + expect(wrapper.get('[data-test="client-preview-confirm-cursor"]').attributes('disabled')).toBeUndefined() + }) }) From 2e932a29e0262a911241b701555875611dde6377 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 5 Oct 2026 15:04:11 +0300 Subject: [PATCH 3/4] fix(web): show why Connect is disabled after a failed preview re-fetch, clear error on Cancel, keep Connect disabled when reopen fetch fails (refs #1446) --- frontend/src/components/ClientConnectList.vue | 16 ++++++++-- .../unit/clients-stale-async-1446.spec.ts | 29 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/ClientConnectList.vue b/frontend/src/components/ClientConnectList.vue index cdb91e2c9..fe21f56b5 100644 --- a/frontend/src/components/ClientConnectList.vue +++ b/frontend/src/components/ClientConnectList.vue @@ -276,6 +276,13 @@ {{ conflictOf(client.id)!.message }} Revoke or delete token {{ conflictOf(client.id)!.conflicting_token }}, then connect again.
+ +

{{ previewError[client.id] }}