mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: free a disposed session's MCP tools and mark a reloaded transcript
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgzuxyafKNF2uaEeL35XQw
This commit is contained in:
co-authored by
Claude Opus 5
parent
5be39c27c6
commit
6d2eaa5621
@@ -41,6 +41,7 @@
|
||||
import ExpandableImage from '$lib/components/common/image/ExpandableImage.svelte'
|
||||
import McpServerIcon from '$lib/components/mcp/McpServerIcon.svelte'
|
||||
import { resolveMcpServerMark } from '$lib/components/mcp/serverMark'
|
||||
import { cachedProviderMark } from '$lib/components/mcp/iconCache'
|
||||
import { mcpServerForToolName } from './global/mcpTools'
|
||||
|
||||
interface Props {
|
||||
@@ -64,10 +65,18 @@
|
||||
}
|
||||
return mcpServerForToolName(aiChatManager.mcpOwnerId, name)
|
||||
})()
|
||||
if (!claimed) return undefined
|
||||
// Both sources are ultimately a path the model wrote, and resolving one reads
|
||||
// that resource and asks a third party for its host's favicon. Only a server
|
||||
// the user actually connected gets marked.
|
||||
return claimed && aiChatManager.mcpServers.some((s) => s.path === claimed) ? claimed : undefined
|
||||
// that resource and asks a third party for its host's favicon — so it has to be
|
||||
// a server the user connected, not any path the model can name.
|
||||
if (aiChatManager.mcpServers.some((s) => s.path === claimed)) return claimed
|
||||
// A session runtime does not populate `mcpServers` until its first send, so on a
|
||||
// reloaded transcript the live list is empty and the rows this path was persisted
|
||||
// for would go unmarked. The local icon cache is the second witness: it only ever
|
||||
// holds servers this user's own MCP list resolved, and answering from it costs
|
||||
// neither a read nor a request.
|
||||
const workspace = aiChatManager.operatingWorkspace
|
||||
return workspace && cachedProviderMark(workspace, claimed) ? claimed : undefined
|
||||
})
|
||||
|
||||
const isPlanReview = $derived(message.toolName === EXIT_PLAN_MODE_TOOL)
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { providerHost } from './iconCache'
|
||||
|
||||
/**
|
||||
* `providerHost` decides which hostnames are sent to a third-party favicon service.
|
||||
* A regression here does not fail visibly — it silently starts disclosing an
|
||||
* endpoint — so the cases it withholds are pinned.
|
||||
*/
|
||||
describe('providerHost', () => {
|
||||
it('returns the host of a public server url', () => {
|
||||
expect(providerHost('https://mcp.linear.app/mcp')).toBe('mcp.linear.app')
|
||||
})
|
||||
|
||||
it('withholds loopback and bare addresses', () => {
|
||||
expect(providerHost('http://localhost:3000/mcp')).toBeUndefined()
|
||||
expect(providerHost('http://127.0.0.1:8000/mcp')).toBeUndefined()
|
||||
expect(providerHost('http://10.1.2.3/mcp')).toBeUndefined()
|
||||
// `new URL()` brackets an IPv6 literal, which has no dot to split on.
|
||||
expect(providerHost('http://[::1]:8000/mcp')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('withholds intranet names', () => {
|
||||
expect(providerHost('https://mcp-box/mcp')).toBeUndefined()
|
||||
expect(providerHost('https://mcp.acme.internal/mcp')).toBeUndefined()
|
||||
expect(providerHost('https://Tools.Acme.LOCAL/mcp')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns nothing for a value that is not a url', () => {
|
||||
expect(providerHost(undefined)).toBeUndefined()
|
||||
expect(providerHost('not a url')).toBeUndefined()
|
||||
expect(providerHost({ url: 'https://example.com' })).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -2,6 +2,7 @@ import { SvelteMap } from 'svelte/reactivity'
|
||||
import { get } from 'svelte/store'
|
||||
import { base } from '$lib/base'
|
||||
import { AIChatManager, AIMode } from '$lib/components/copilot/chat/AIChatManager.svelte'
|
||||
import { forgetLoadedMcpTools } from '$lib/components/copilot/chat/global/mcpTools'
|
||||
import { PipelineEditorState } from '$lib/components/assets/AssetGraph/pipelineEditorState.svelte'
|
||||
import { initFlow } from '$lib/components/flows/flowStore.svelte'
|
||||
import {
|
||||
@@ -966,6 +967,10 @@ export function disposeRuntime(sessionId: string) {
|
||||
if (!runtime) return
|
||||
runtime.manager.cancel('runtime disposed')
|
||||
runtime.manager.historyManager.close()
|
||||
// The registry is keyed by the manager's own id, so nothing else can ever reach
|
||||
// this entry once the runtime is gone — it would hold its remote tool schemas for
|
||||
// the life of the page.
|
||||
forgetLoadedMcpTools(runtime.manager.mcpOwnerId)
|
||||
runtimes.delete(sessionId)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user