diff --git a/.github/workflows/plugin-dev-host.yml b/.github/workflows/plugin-dev-host.yml index 4aab18ea15..b2ddec1cce 100644 --- a/.github/workflows/plugin-dev-host.yml +++ b/.github/workflows/plugin-dev-host.yml @@ -32,6 +32,7 @@ jobs: - run: npm run lint --prefix plugins/sdk/dev-host - run: npm run fmt:check --prefix plugins/sdk/dev-host - run: npm test --prefix plugins/sdk/dev-host + timeout-minutes: 5 - run: cargo test --locked --manifest-path plugins/sdk/cli/Cargo.toml - run: npm test --prefix packages/plugin-cli - run: node scripts/verify-plugin-cli-package.mjs diff --git a/apps/desktop/src/components/connection/ConnectionDialog.vue b/apps/desktop/src/components/connection/ConnectionDialog.vue index 4d0e38d1dd..7716b196b2 100644 --- a/apps/desktop/src/components/connection/ConnectionDialog.vue +++ b/apps/desktop/src/components/connection/ConnectionDialog.vue @@ -56,7 +56,16 @@ import DatabaseIcon from "@/components/icons/DatabaseIcon.vue"; import PluginConnectionFields from "@/components/plugins/PluginConnectionFields.vue"; import PluginIcon from "@/components/plugins/PluginIcon.vue"; import * as api from "@/lib/backend/api"; -import { buildPluginConnectionConfig, createFrontendPluginRegistry, parsePluginConnectionProviderOptionValue, pluginConnectionActionsForDialog, pluginConnectionFormValues, pluginConnectionProviderIcon, pluginConnectionProviderOptionValue } from "@/lib/plugins/frontendPlugin"; +import { + buildPluginConnectionConfig, + createFrontendPluginRegistry, + parsePluginConnectionProviderOptionValue, + pluginConnectionActionsForDialog, + pluginConnectionConnectTimeoutDefault, + pluginConnectionFormValues, + pluginConnectionProviderIcon, + pluginConnectionProviderOptionValue, +} from "@/lib/plugins/frontendPlugin"; import type { PluginCenterFocus } from "@/lib/plugins/pluginCenterNavigation"; import { isTauriRuntime } from "@/lib/backend/tauriRuntime"; import { applyMeilisearchBasePathToExternalConfig, applyParsedConnectionUrl, normalizeMongoConnectionString, parseConnectionUrl } from "@/lib/connection/connectionUrl"; @@ -4099,12 +4108,26 @@ function connectionConfigForSubmit(id: string, generatedName = "", validatePlugi } const existing = props.editConfig?.db_type === "plugin" && props.editConfig.plugin_id === entry.plugin.manifest.id && props.editConfig.plugin_connection_provider === entry.contribution.id ? props.editConfig : undefined; config = buildPluginConnectionConfig(entry.plugin.manifest.id, entry.contribution, values, existing) as LegacyConnectionConfig; + // buildPluginConnectionConfig already mirrored the provider's resolved + // connect_timeout_secs (declared default or advanced-form value) into the + // typed field; capture it before the generic form overwrite below. + const resolvedPluginConnectTimeout = pluginConnectionConnectTimeoutDefault(entry.contribution) === undefined ? undefined : config.connect_timeout_secs; config.id = id; config.name = form.value.name.trim() || config.name; config.note = form.value.note; config.color = form.value.color; config.transport_layers = form.value.transport_layers || []; config.connect_timeout_secs = form.value.connect_timeout_secs; + if (resolvedPluginConnectTimeout !== undefined) { + // A provider declaring its own connect_timeout_secs field makes it the + // single source of truth (declared default or advanced-form value): the + // typed timeout mirrors it, and the generic global/per-connection DBX + // timeout radios do not apply. Otherwise the host RPC deadline and the + // plugin's own handshake timeout could disagree and the host would kill + // slow connects first. + config.connect_timeout_secs = resolvedPluginConnectTimeout; + config.connect_timeout_inherit = false; + } config.query_timeout_secs = form.value.query_timeout_secs; config.idle_timeout_secs = form.value.idle_timeout_secs; config.keepalive_interval_secs = form.value.keepalive_interval_secs; diff --git a/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts b/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts new file mode 100644 index 0000000000..5d49e22b28 --- /dev/null +++ b/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts @@ -0,0 +1,135 @@ +// @vitest-environment happy-dom + +import { createApp, nextTick, ref, type App } from "vue"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { InstalledPlugin, PluginWorkbenchContribution } from "@/types/database"; + +const mocks = vi.hoisted(() => ({ + readPluginUiEntry: vi.fn(), + readPluginUiAsset: vi.fn(), + subscribePluginEvents: vi.fn(), + repushPluginConnection: vi.fn(), + reopenPluginConnection: vi.fn(), +})); + +vi.mock("@/lib/backend/api", () => ({ + invokePlugin: vi.fn(), + notifyPlugin: vi.fn(), + sendPluginBinary: vi.fn(), + readPluginUiEntry: mocks.readPluginUiEntry, + readPluginUiAsset: mocks.readPluginUiAsset, + subscribePluginEvents: mocks.subscribePluginEvents, +})); +vi.mock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); +vi.mock("@/lib/common/clipboard", () => ({ copyToClipboard: vi.fn() })); +vi.mock("@/composables/useTheme", () => ({ useTheme: () => ({ isDark: ref(false), themeRevision: ref(0) }) })); +vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ editorSettings: { uiFontFamily: "", fontFamily: "", fontSize: 14 } }) })); +vi.mock("@/stores/connectionStore", () => ({ + useConnectionStore: () => ({ + repushPluginConnection: mocks.repushPluginConnection, + reopenPluginConnection: mocks.reopenPluginConnection, + }), +})); +vi.mock("vue-i18n", () => ({ useI18n: () => ({ locale: ref("en"), t: (key: string) => key }) })); + +import PluginWorkbenchHost from "./PluginWorkbenchHost.vue"; + +const plugin: InstalledPlugin = { + manifest: { id: "sample", name: "Sample", version: "1.0.0", permissions: [], drivers: [], contributions: [] }, + compatibility: { compatible: true }, +}; +const contribution: PluginWorkbenchContribution = { type: "workbench", id: "sample.main", label: "Sample" }; + +function deferred() { + let resolve!: () => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +async function flushWorkbenchLoad() { + await nextTick(); + await new Promise((resolve) => setTimeout(resolve, 0)); + await nextTick(); +} + +describe("PluginWorkbenchHost initialization", () => { + let app: App | undefined; + let root: HTMLDivElement; + + beforeEach(() => { + vi.clearAllMocks(); + vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => window.setTimeout(() => callback(0), 0)); + vi.stubGlobal("getComputedStyle", () => Object.assign([], { getPropertyValue: () => "" })); + mocks.readPluginUiEntry.mockResolvedValue({ dataBase64: btoa(""), contentType: "text/html" }); + mocks.subscribePluginEvents.mockResolvedValue(vi.fn()); + mocks.repushPluginConnection.mockReset().mockResolvedValue(undefined); + mocks.reopenPluginConnection.mockResolvedValue(undefined); + root = document.createElement("div"); + document.body.appendChild(root); + }); + + afterEach(() => { + app?.unmount(); + root.remove(); + vi.unstubAllGlobals(); + }); + + async function mountHost() { + app = createApp(PluginWorkbenchHost, { plugin, contribution, context: { connectionId: "connection" } }); + app.mount(root); + await flushWorkbenchLoad(); + const frame = root.querySelector("iframe"); + expect(frame).toBeInstanceOf(HTMLIFrameElement); + const target = frame!.contentWindow!; + const postMessage = vi.spyOn(target, "postMessage").mockImplementation(() => {}); + const ready = () => window.dispatchEvent(new MessageEvent("message", { source: target, data: { source: "dbx-plugin", version: 1, type: "ready" } })); + ready(); + await vi.waitFor(() => expect(mocks.repushPluginConnection).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 0)); + mocks.repushPluginConnection.mockClear(); + postMessage.mockClear(); + return { frame: frame!, postMessage, ready }; + } + + it("runs reinit before one init when load precedes ready", async () => { + const firstReinit = deferred(); + const { frame, postMessage, ready } = await mountHost(); + mocks.repushPluginConnection.mockReturnValue(firstReinit.promise); + + frame.dispatchEvent(new Event("load")); + ready(); + + expect(mocks.repushPluginConnection).toHaveBeenCalledTimes(1); + expect(postMessage).not.toHaveBeenCalled(); + firstReinit.resolve(); + await vi.waitFor(() => expect(postMessage).toHaveBeenCalledTimes(1)); + expect(postMessage.mock.calls[0]?.[0]).toMatchObject({ type: "init" }); + + const secondReinit = deferred(); + mocks.repushPluginConnection.mockReturnValue(secondReinit.promise); + frame.dispatchEvent(new Event("load")); + ready(); + + expect(mocks.repushPluginConnection).toHaveBeenCalledTimes(2); + expect(postMessage).toHaveBeenCalledTimes(1); + secondReinit.resolve(); + await vi.waitFor(() => expect(postMessage).toHaveBeenCalledTimes(2)); + }); + + it("runs reinit before one init when ready precedes load", async () => { + const reinit = deferred(); + const { frame, postMessage, ready } = await mountHost(); + mocks.repushPluginConnection.mockReturnValue(reinit.promise); + + ready(); + frame.dispatchEvent(new Event("load")); + + expect(mocks.repushPluginConnection).toHaveBeenCalledTimes(1); + expect(postMessage).not.toHaveBeenCalled(); + reinit.resolve(); + await vi.waitFor(() => expect(postMessage).toHaveBeenCalledTimes(1)); + expect(postMessage.mock.calls[0]?.[0]).toMatchObject({ type: "init" }); + }); +}); diff --git a/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue b/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue index 5b8dc240af..5ee1b8fc17 100644 --- a/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue +++ b/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue @@ -8,6 +8,8 @@ import { PluginHostBridge, pluginSandboxDocument, type PluginBridgeTheme, type P import type { InstalledPlugin, PluginWorkbenchContribution } from "@/types/database"; import { useI18n } from "vue-i18n"; import { useTheme } from "@/composables/useTheme"; +import { useSettingsStore } from "@/stores/settingsStore"; +import { useConnectionStore } from "@/stores/connectionStore"; const props = withDefaults( defineProps<{ @@ -21,13 +23,14 @@ const props = withDefaults( const emit = defineEmits<{ ready: []; error: [message: string]; - openWorkbench: [pluginId: string, contributionId: string, context?: PluginWorkbenchContext]; + openWorkbench: [pluginId: string, contributionId: string, context?: PluginWorkbenchContext, options?: { forceNew?: boolean }]; openFilesystem: [pluginId: string, providerId: string, context?: PluginWorkbenchContext]; closeTab: []; }>(); const { t, locale: appLocale } = useI18n(); const { isDark, themeRevision } = useTheme(); +const settingsStore = useSettingsStore(); const iframe = ref(); const source = ref(""); const loading = ref(true); @@ -70,8 +73,9 @@ function createBridge() { notify: api.notifyPlugin, sendBinary: api.sendPluginBinary, readAsset: api.readPluginUiAsset, - openWorkbench: async (pluginId, contributionId, context) => emit("openWorkbench", pluginId, contributionId, context), + openWorkbench: async (pluginId, contributionId, context, options) => emit("openWorkbench", pluginId, contributionId, context, options), openFilesystem: async (pluginId, providerId, context) => emit("openFilesystem", pluginId, providerId, context), + reopenConnection: (pluginId, connectionId) => useConnectionStore().reopenPluginConnection(connectionId, pluginId), closeTab: () => emit("closeTab"), saveFile: (_pluginId, request, data) => savePluginFile(request, data), copyText: (_pluginId, text) => copyToClipboard(text), @@ -79,6 +83,16 @@ function createBridge() { appLocale.value, currentBridgeTheme(), ); + // An iframe reload (F5 / webview restart) drops the plugin sidecar's + // in-memory connection registry while the host still holds the connection + // open. Re-push the connection config through the same path as a sidebar + // open before the plugin receives its fresh init, so it can reconnect + // without the user reopening the connection from the sidebar. + bridge.onReinit = async () => { + const connectionId = props.context?.connectionId; + if (!connectionId) return; + await useConnectionStore().repushPluginConnection(connectionId); + }; } /** Keep a plugin-supplied name from smuggling path separators or traversal into the save dialog. */ @@ -161,6 +175,7 @@ async function inlineLocalUiAssets(html: string, pluginId: string): Promise bridge?.updateLocale(locale)); // path — dark/light, palette switch, custom colors — re-pushes the resolved // tokens; watching isDark/custom colors alone misses palette-only switches. watch(themeRevision, () => bridge?.updateTheme(currentBridgeTheme())); +// Font settings are applied outside applyTheme() (see App.vue applyUiFontFamily) +// and therefore never bump themeRevision; watch them explicitly so font token +// changes reach live plugin bridges without waiting for the next theme switch. +watch( + () => [settingsStore.editorSettings.uiFontFamily, settingsStore.editorSettings.fontFamily, settingsStore.editorSettings.fontSize], + () => bridge?.updateTheme(currentBridgeTheme()), +); onBeforeUnmount(() => { disposed = true; loadGeneration += 1; + bridge?.dispose(); bridge = undefined; window.removeEventListener("message", onMessage); unsubscribeEvents?.(); diff --git a/apps/desktop/src/components/plugins/PluginWorkbenchTab.vue b/apps/desktop/src/components/plugins/PluginWorkbenchTab.vue index 0c8db1b1df..90c75452b8 100644 --- a/apps/desktop/src/components/plugins/PluginWorkbenchTab.vue +++ b/apps/desktop/src/components/plugins/PluginWorkbenchTab.vue @@ -30,6 +30,12 @@ const error = ref(""); const deferred = ref(false); let loadGeneration = 0; const resolvedConnectionId = computed(() => props.connectionId || (typeof props.context?.connectionId === "string" ? props.context.connectionId : undefined)); +// The host's re-push-on-reinit and the plugin itself both key off +// context.connectionId; filesystem tabs carry it only as a tab-level prop. +const hostContext = computed(() => { + if (!resolvedConnectionId.value || props.context?.connectionId) return props.context; + return { ...props.context, connectionId: resolvedConnectionId.value }; +}); const entry = computed(() => createFrontendPluginRegistry(plugins.value, appLocale.value).findWorkbench(props.pluginId, props.contributionId)); async function load() { @@ -54,7 +60,18 @@ async function refresh() { loading.value = true; error.value = ""; try { - if (resolvedConnectionId.value) await connectionStore.ensureConnected(resolvedConnectionId.value); + const connectionId = resolvedConnectionId.value; + if (connectionId) { + if (connectionStore.connectedIds.has(connectionId)) { + // The connection is open but the plugin sidecar's registry may have + // been wiped (iframe reload, tab restore): ensureConnected's health + // fast-path only probes the sidecar process and would not heal that. + // Force a re-push instead; it no-ops when a push just happened. + await connectionStore.repushPluginConnection(connectionId); + } else { + await connectionStore.ensureConnected(connectionId); + } + } await load(); } catch (cause) { loading.value = false; @@ -73,9 +90,14 @@ function start() { void load(); } -function openWorkbench(pluginId: string, contributionId: string, context?: PluginWorkbenchContext) { +function openWorkbench(pluginId: string, contributionId: string, context?: PluginWorkbenchContext, options?: { forceNew?: boolean }) { const target = createFrontendPluginRegistry(plugins.value, appLocale.value).findWorkbench(pluginId, contributionId); - queryStore.openPluginWorkbench(pluginId, contributionId, { title: target?.contribution.label || contributionId, context }); + // Session tabs are per-connection: title them after the connection (like the + // sidebar-opened tab) so parallel sessions read "server", "server (2)", … + // — the numbering itself is applied in queryStore.openPluginWorkbench. + const contextConnectionId = typeof context?.connectionId === "string" ? context.connectionId : ""; + const connectionName = contextConnectionId ? connectionStore.getConfig(contextConnectionId)?.name : undefined; + queryStore.openPluginWorkbench(pluginId, contributionId, { title: connectionName || target?.contribution.label || contributionId, context, forceNew: options?.forceNew === true }); } function openFilesystem(pluginId: string, providerId: string, context?: PluginWorkbenchContext) { @@ -105,6 +127,6 @@ defineExpose({ refresh }); {{ error || t("pluginPlatform.workbenchUnavailableFallback") }} - + diff --git a/apps/desktop/src/lib/plugins/frontendPlugin.spec.ts b/apps/desktop/src/lib/plugins/frontendPlugin.spec.ts index dea3dd11b5..cf258b054e 100644 --- a/apps/desktop/src/lib/plugins/frontendPlugin.spec.ts +++ b/apps/desktop/src/lib/plugins/frontendPlugin.spec.ts @@ -1,6 +1,16 @@ import { describe, expect, it } from "vitest"; import type { InstalledPlugin, PluginConnectionProviderContribution, PluginFormFieldValue } from "@/types/database"; -import { buildPluginConnectionConfig, createFrontendPluginRegistry, initialPluginFormValues, parsePluginConnectionProviderOptionValue, pluginConnectionActionsForDialog, pluginConnectionFormValues, pluginConnectionProviderIcon, pluginConnectionProviderOptionValue } from "./frontendPlugin"; +import { + buildPluginConnectionConfig, + createFrontendPluginRegistry, + initialPluginFormValues, + parsePluginConnectionProviderOptionValue, + pluginConnectionActionsForDialog, + pluginConnectionConnectTimeoutDefault, + pluginConnectionFormValues, + pluginConnectionProviderIcon, + pluginConnectionProviderOptionValue, +} from "./frontendPlugin"; function installedPlugin(id: string, contributions: InstalledPlugin["manifest"]["contributions"] = []): InstalledPlugin { return { @@ -70,6 +80,28 @@ describe("FrontendPluginRegistry", () => { expect(transient.save_password).toBe(false); }); + it("materializes the provider's connect_timeout_secs default into the typed timeout", () => { + const provider = connectionProvider({ + fields: [{ key: "connect_timeout_secs", label: "Connect timeout", type: "number", default: 30 }], + }); + expect(pluginConnectionConnectTimeoutDefault(provider)).toBe(30); + expect(buildPluginConnectionConfig("example.plugin", provider, {}).connect_timeout_secs).toBe(30); + // An advanced-form value the user tuned is mirrored into the typed field too. + expect(buildPluginConnectionConfig("example.plugin", provider, { connect_timeout_secs: 60 }).connect_timeout_secs).toBe(60); + + // Once the provider declares the field, its resolved value is the single + // source of truth: a stale typed value from before the declaration is + // healed to the declared default. + const existing = buildPluginConnectionConfig("example.plugin", connectionProvider({ fields: [] }), {}); + existing.connect_timeout_secs = 7; + expect(buildPluginConnectionConfig("example.plugin", provider, {}, existing).connect_timeout_secs).toBe(30); + + // Providers without the well-known field keep the generic 10s default. + const generic = connectionProvider({ fields: [] }); + expect(pluginConnectionConnectTimeoutDefault(generic)).toBeUndefined(); + expect(buildPluginConnectionConfig("example.plugin", generic, {}).connect_timeout_secs).toBe(10); + }); + it("round-trips plugin connection provider picker values", () => { const value = pluginConnectionProviderOptionValue("example/plugin", "ssh:main"); expect(parsePluginConnectionProviderOptionValue(value)).toEqual({ pluginId: "example/plugin", providerId: "ssh:main" }); diff --git a/apps/desktop/src/lib/plugins/frontendPlugin.ts b/apps/desktop/src/lib/plugins/frontendPlugin.ts index 774a9de625..2f371531dc 100644 --- a/apps/desktop/src/lib/plugins/frontendPlugin.ts +++ b/apps/desktop/src/lib/plugins/frontendPlugin.ts @@ -105,6 +105,22 @@ export function pluginConnectionProviderIcon(entry: PluginContributionEntry candidate.key === PLUGIN_CONNECT_TIMEOUT_FIELD_KEY && effectiveFieldBinding(candidate) === "config"); + const value = field?.default; + if (typeof value !== "number" || !Number.isFinite(value) || value <= 0) return undefined; + return Math.min(300, Math.max(1, Math.trunc(value))); +} + export function pluginConnectionActionsForDialog(contribution: PluginConnectionProviderContribution, editing: boolean): PluginConnectionAction[] { const actions: PluginConnectionAction[] = [ ...(contribution.actions || []).map((action) => ({ ...action, kind: "custom" as const })), @@ -159,7 +175,7 @@ export function buildPluginConnectionConfig(pluginId: string, contribution: Plug plugin_connection_type: contribution.database_type, connection_secrets: connectionSecrets, transport_layers: existing?.transport_layers || [], - connect_timeout_secs: existing?.connect_timeout_secs || 10, + connect_timeout_secs: existing?.connect_timeout_secs || pluginConnectionConnectTimeoutDefault(contribution) || 10, query_timeout_secs: existing?.query_timeout_secs || 60, idle_timeout_secs: existing?.idle_timeout_secs || 60, keepalive_interval_secs: existing?.keepalive_interval_secs || 30, @@ -198,6 +214,17 @@ export function buildPluginConnectionConfig(pluginId: string, contribution: Plug config.database = value === undefined || value === "" ? undefined : String(value); } } + // A config-bound connect_timeout_secs field is the plugin's own handshake + // timeout (the SSH plugin lets advanced users tune it). Mirror the resolved + // value into the typed field so the host RPC deadline never fires before the + // plugin's own timeout. Only applies while the provider declares the field — + // a stale external_config key from an older manifest must not leak through. + if (pluginConnectionConnectTimeoutDefault(contribution) !== undefined) { + const pluginConnectTimeout = externalConfig[PLUGIN_CONNECT_TIMEOUT_FIELD_KEY]; + if (typeof pluginConnectTimeout === "number" && Number.isFinite(pluginConnectTimeout) && pluginConnectTimeout > 0) { + config.connect_timeout_secs = Math.min(300, Math.max(1, Math.trunc(pluginConnectTimeout))); + } + } return config; } diff --git a/apps/desktop/src/lib/plugins/pluginHostBridge.spec.ts b/apps/desktop/src/lib/plugins/pluginHostBridge.spec.ts index 82cc2aa5cc..334d77c973 100644 --- a/apps/desktop/src/lib/plugins/pluginHostBridge.spec.ts +++ b/apps/desktop/src/lib/plugins/pluginHostBridge.spec.ts @@ -58,6 +58,55 @@ describe("PluginHostBridge", () => { expect(messages[0]).toMatchObject({ source: "dbx-host", type: "init", locale: "zh-CN", context: { connectionId: "connection" } }); }); + it("runs onReinit before the init message when the iframe reloads", async () => { + const messages: unknown[] = []; + const target = { postMessage: (message: unknown) => messages.push(message) } as unknown as Window; + const bridge = new PluginHostBridge(plugin(), workbench, {}, () => target, { + invoke: vi.fn(), + notify: vi.fn(), + sendBinary: vi.fn(), + readAsset: vi.fn(), + }); + const ready = () => bridge.handleWindowMessage({ source: target, data: { source: "dbx-plugin", version: 1, type: "ready" } } as MessageEvent); + + const order: string[] = []; + bridge.onReinit = () => + new Promise((resolve) => { + order.push("reinit"); + setTimeout(resolve, 5); + }); + const originalPost = target.postMessage.bind(target); + (target as { postMessage: (m: unknown) => void }).postMessage = (message: unknown) => { + if ((message as { type?: string }).type === "init") order.push("init"); + originalPost(message); + }; + + expect(ready()).toBe(true); + await vi.waitFor(() => expect(messages).toHaveLength(1)); + expect(order).toEqual(["reinit", "init"]); + }); + + it("initializes only once when ready is repeated for the same bridge", async () => { + const messages: unknown[] = []; + const target = { postMessage: (message: unknown) => messages.push(message) } as unknown as Window; + const bridge = new PluginHostBridge(plugin(), workbench, {}, () => target, { + invoke: vi.fn(), + notify: vi.fn(), + sendBinary: vi.fn(), + readAsset: vi.fn(), + }); + const onReinit = vi.fn(); + bridge.onReinit = onReinit; + const ready = () => bridge.handleWindowMessage({ source: target, data: { source: "dbx-plugin", version: 1, type: "ready" } } as MessageEvent); + + ready(); + await vi.waitFor(() => expect(messages).toHaveLength(1)); + ready(); + + expect(onReinit).toHaveBeenCalledTimes(1); + expect(messages).toHaveLength(1); + }); + it("snapshots nested Vue reactive context values before sending them to the plugin", () => { const messages: unknown[] = []; const target = { postMessage: (message: unknown) => messages.push(message) } as unknown as Window; @@ -135,6 +184,69 @@ describe("PluginHostBridge", () => { expect(messages[0]).toMatchObject({ id: "2", error: "Plugin has not declared permission 'host.workbench'" }); }); + it("passes forceNew through host.openWorkbench and defaults it to false", async () => { + const messages: unknown[] = []; + const target = { postMessage: (message: unknown) => messages.push(message) } as unknown as Window; + const openWorkbench = vi.fn(); + const bridge = new PluginHostBridge(plugin(["host.workbench"]), workbench, {}, () => target, { + invoke: vi.fn(), + notify: vi.fn(), + sendBinary: vi.fn(), + readAsset: vi.fn(), + openWorkbench, + }); + const request = (id: string, params: unknown) => + bridge.handleWindowMessage({ + source: target, + data: { source: "dbx-plugin", version: 1, type: "request", id, method: "host.openWorkbench", params }, + } as MessageEvent); + + request("new", { contributionId: "sample.other", context: { connectionId: "c1" }, forceNew: true }); + await vi.waitFor(() => expect(messages).toHaveLength(1)); + expect(openWorkbench).toHaveBeenNthCalledWith(1, "sample", "sample.other", { connectionId: "c1" }, { forceNew: true }); + expect(messages[0]).toMatchObject({ id: "new", result: null }); + + request("default", { contributionId: "sample.other" }); + await vi.waitFor(() => expect(messages).toHaveLength(2)); + expect(openWorkbench).toHaveBeenNthCalledWith(2, "sample", "sample.other", undefined, { forceNew: false }); + }); + + it("reopens a plugin connection through host.reopenConnection", async () => { + const messages: unknown[] = []; + const target = { postMessage: (message: unknown) => messages.push(message) } as unknown as Window; + const reopenConnection = vi.fn().mockResolvedValue(undefined); + const bridge = new PluginHostBridge(plugin(), workbench, {}, () => target, { + invoke: vi.fn(), + notify: vi.fn(), + sendBinary: vi.fn(), + readAsset: vi.fn(), + reopenConnection, + }); + const request = (id: string, params: unknown) => + bridge.handleWindowMessage({ + source: target, + data: { source: "dbx-plugin", version: 1, type: "request", id, method: "host.reopenConnection", params }, + } as MessageEvent); + + request("reopen", { connectionId: "9f1c2a34-0000-4000-8000-abcdef012345" }); + await vi.waitFor(() => expect(messages).toHaveLength(1)); + expect(reopenConnection).toHaveBeenCalledWith("sample", "9f1c2a34-0000-4000-8000-abcdef012345"); + expect(messages[0]).toMatchObject({ id: "reopen", result: { ok: true } }); + + // Missing/invalid connectionId is rejected before reaching the host. + request("missing", {}); + await vi.waitFor(() => expect(messages).toHaveLength(2)); + expect(messages[1]).toMatchObject({ id: "missing" }); + expect((messages[1] as { error?: string }).error).toContain("connectionId"); + + // Host-side failures (connection missing, not plugin-backed, connect + // failed) surface as the request error so the plugin can show them. + reopenConnection.mockRejectedValueOnce(new Error("Connection is not plugin-backed")); + request("not-plugin", { connectionId: "mysql-1" }); + await vi.waitFor(() => expect(messages).toHaveLength(3)); + expect(messages[2]).toMatchObject({ id: "not-plugin", error: "Connection is not plugin-backed" }); + }); + it("opens only the owning plugin filesystem with explicit permission", async () => { const messages: unknown[] = []; const target = { postMessage: (message: unknown) => messages.push(message) } as unknown as Window; diff --git a/apps/desktop/src/lib/plugins/pluginHostBridge.ts b/apps/desktop/src/lib/plugins/pluginHostBridge.ts index 1e0c773120..6672a0b63e 100644 --- a/apps/desktop/src/lib/plugins/pluginHostBridge.ts +++ b/apps/desktop/src/lib/plugins/pluginHostBridge.ts @@ -38,8 +38,10 @@ export interface PluginHostBridgeApi { notify(pluginId: string, method: string, params?: unknown): Promise; sendBinary(pluginId: string, channel: string, dataBase64: string): Promise; readAsset(pluginId: string, path: string): Promise; - openWorkbench?(pluginId: string, contributionId: string, context?: PluginWorkbenchContext): Promise | void; + openWorkbench?(pluginId: string, contributionId: string, context?: PluginWorkbenchContext, options?: { forceNew?: boolean }): Promise | void; openFilesystem?(pluginId: string, providerId: string, context?: PluginWorkbenchContext): Promise | void; + /** Explicit user-triggered reconnect of an owned plugin connection (full flow, interactive password prompt allowed). */ + reopenConnection?(pluginId: string, connectionId: string): Promise; closeTab?(): Promise | void; /** Persist plugin bytes through the host's native save dialog. Resolves null when the user cancels. */ saveFile?(pluginId: string, request: PluginSaveFileRequest, data: Uint8Array): Promise; @@ -63,6 +65,9 @@ export class PluginHostBridge { private locale: string; private theme?: PluginBridgeTheme; + /** Invoked once before each iframe load generation sends its init message. */ + onReinit?: () => Promise | void; + constructor( private readonly plugin: InstalledPlugin, private readonly workbench: PluginWorkbenchContribution, @@ -82,7 +87,7 @@ export class PluginHostBridge { if (!target || event.source !== target || !isRecord(event.data)) return false; if (event.data.source !== PLUGIN_MESSAGE_SOURCE || event.data.version !== BRIDGE_VERSION) return false; if (event.data.type === "ready") { - this.sendInit(); + void this.handleReady(); return true; } if (event.data.type === "shortcut" && event.data.shortcut === "closeTab") { @@ -94,7 +99,52 @@ export class PluginHostBridge { return true; } + private handleReady(): void { + this.requestInit("ready"); + } + sendInit(): void { + this.requestInit("load"); + } + + private requestInit(signal: "load" | "ready"): void { + if ((this.initSignals.load && this.initSignals.ready) || (signal === "load" && this.initSignals.load)) { + this.initGeneration += 1; + this.initSignals = { load: false, ready: false }; + this.initStarted = false; + } + if (this.initSignals[signal]) return; + this.initSignals[signal] = true; + if (this.initStarted) return; + this.initStarted = true; + const generation = this.initGeneration; + const reinit = this.onReinit; + if (!reinit) { + if (!this.disposed && generation === this.initGeneration) this.postInit(); + return; + } + void (async () => { + try { + await reinit(); + } catch (error) { + console.warn("[DBX][plugin-bridge:reinit]", error); + } + // A newer load generation supersedes this one; never post a stale init. + if (!this.disposed && generation === this.initGeneration) this.postInit(); + })(); + } + + /** Stop future posts (queued inits after an async reinit) for a torn-down bridge. */ + dispose(): void { + this.disposed = true; + } + + private disposed = false; + private initGeneration = 0; + private initSignals = { load: false, ready: false }; + private initStarted = false; + + private postInit(): void { this.post({ source: HOST_MESSAGE_SOURCE, version: BRIDGE_VERSION, @@ -186,9 +236,15 @@ export class PluginHostBridge { this.requirePermission("host.workbench"); if (!this.api.openWorkbench) throw new Error("Host workbench navigation is unavailable"); const input = requireRecord(params, "host.openWorkbench params"); - await this.api.openWorkbench(this.plugin.manifest.id, requireProtocolName(input.contributionId, "workbench contribution"), isRecord(input.context) ? input.context : undefined); + await this.api.openWorkbench(this.plugin.manifest.id, requireProtocolName(input.contributionId, "workbench contribution"), isRecord(input.context) ? input.context : undefined, { forceNew: input.forceNew === true }); return null; } + if (method === "host.reopenConnection") { + if (!this.api.reopenConnection) throw new Error("Connection reopen is unavailable"); + const input = requireRecord(params, "host.reopenConnection params"); + await this.api.reopenConnection(this.plugin.manifest.id, requireProtocolName(input.connectionId, "connectionId")); + return { ok: true }; + } if (method === "host.openFilesystem") { this.requirePermission("host.filesystem"); if (!this.api.openFilesystem) throw new Error("Host filesystem navigation is unavailable"); @@ -498,8 +554,9 @@ export function pluginSdkSource(initialTheme?: PluginBridgeTheme): string { const asset = await request('ui.readAsset', { path }); return URL.createObjectURL(new Blob([decode(asset.dataBase64)], { type: asset.contentType })); }, - openWorkbench: (contributionId, childContext) => request('host.openWorkbench', { contributionId, context: childContext }), + openWorkbench: (contributionId, childContext, options) => request('host.openWorkbench', { contributionId, context: childContext, forceNew: !!(options && options.forceNew) }), openFilesystem: (providerId, childContext) => request('host.openFilesystem', { providerId, context: childContext }), + reopenConnection: (connectionId) => request('host.reopenConnection', { connectionId }), saveFile: (options = {}, data) => { if (data === undefined) return request('host.saveFile', options); if (typeof data === 'string') return request('host.saveFile', { ...(options || {}), dataBase64: data }); diff --git a/apps/desktop/src/stores/__tests__/connectionStore.savePassword.spec.ts b/apps/desktop/src/stores/__tests__/connectionStore.savePassword.spec.ts index 5508a0dbfe..63063e8ee6 100644 --- a/apps/desktop/src/stores/__tests__/connectionStore.savePassword.spec.ts +++ b/apps/desktop/src/stores/__tests__/connectionStore.savePassword.spec.ts @@ -548,6 +548,35 @@ describe("connectionStore plugin password prompt", () => { expect(connectDb).toHaveBeenCalledWith(expect.objectContaining({ id: "ssh-1", external_config: expect.objectContaining({ authentication: "private-key" }) }), expect.any(Number)); }); + it("rejects reopening a connection owned by another plugin before prompting or connecting", async () => { + const connectDb = vi.fn().mockResolvedValue("ssh-1"); + installApiMocks({ connectDb, listPlugins: vi.fn().mockResolvedValue([sshInstalledPlugin()]) }); + installPasswordPromptMock(); + requestPassword.mockResolvedValue({ password: PROMPT_TYPED_PASSWORD, rememberPassword: false }); + const { useConnectionStore } = await import("@/stores/connectionStore"); + const store = useConnectionStore(); + store.connections = [pluginConnection({ external_config: { authentication: "password" } })]; + + await expect(store.reopenPluginConnection("ssh-1", "io.dbx.other")).rejects.toThrow("Connection is owned by another plugin"); + + expect(requestPassword).not.toHaveBeenCalled(); + expect(connectDb).not.toHaveBeenCalled(); + }); + + it("allows the owning plugin to reopen its connection", async () => { + const connectDb = vi.fn().mockResolvedValue("ssh-1"); + installApiMocks({ connectDb, listPlugins: vi.fn().mockResolvedValue([sshInstalledPlugin()]) }); + installPasswordPromptMock(); + const { useConnectionStore } = await import("@/stores/connectionStore"); + const store = useConnectionStore(); + store.connections = [pluginConnection()]; + + await expect(store.reopenPluginConnection("ssh-1", "io.dbx.ssh")).resolves.toBeUndefined(); + + expect(requestPassword).not.toHaveBeenCalled(); + expect(connectDb).toHaveBeenCalledWith(expect.objectContaining({ id: "ssh-1" }), expect.any(Number)); + }); + it("still prompts for a plugin connection whose manifest requires the login password", async () => { installApiMocks({ listPlugins: vi.fn().mockResolvedValue([sshInstalledPlugin()]) }); installPasswordPromptMock(); diff --git a/apps/desktop/src/stores/__tests__/queryStore.openPluginWorkbench.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.openPluginWorkbench.spec.ts index e4a1d9f34c..7b35d0567b 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.openPluginWorkbench.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.openPluginWorkbench.spec.ts @@ -95,4 +95,74 @@ describe("queryStore openPluginWorkbench reuse", () => { // Adoption must not replace the live context (same no-deep-reload rule). expect(queryStore.tabs.find((t) => t.id === id)?.pluginWorkbench?.context).toEqual({ connectionId: "conn-1" }); }); + + it("numbers same-connection session tabs Termius-style and never backfills", async () => { + const queryStore = useQueryStore(); + + // First tab keeps the bare connection name; numbering starts at (1) from + // the second tab on. + const firstId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { + title: "SSH server222", + connectionId: "conn-1", + context: { connectionId: "conn-1" }, + }); + // forceNew is orthogonal to numbering: it skips dedup, the count still grows. + const secondId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { + title: "SSH server222", + connectionId: "conn-1", + context: { connectionId: "conn-1", workbenchId: "wb-2" }, + forceNew: true, + }); + const thirdId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { + title: "SSH server222", + connectionId: "conn-1", + context: { connectionId: "conn-1", workbenchId: "wb-3" }, + forceNew: true, + }); + + const titles = Object.fromEntries(queryStore.tabs.map((tab) => [tab.id, tab.title])); + expect([titles[firstId], titles[secondId], titles[thirdId]]).toEqual(["SSH server222", "SSH server222 (1)", "SSH server222 (2)"]); + + // Closing tab (1) must not renumber (2): existing titles stay stable (no + // backfill, no drift). The next tab advances beyond the highest live suffix + // so two open sessions never share a title. (Removed directly: the stubbed + // window in this harness lacks the timers closeTab's cleanup needs.) + queryStore.tabs = queryStore.tabs.filter((tab) => tab.id !== secondId); + const fourthId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { + title: "SSH server222", + connectionId: "conn-1", + context: { connectionId: "conn-1", workbenchId: "wb-4" }, + forceNew: true, + }); + expect(queryStore.tabs.find((tab) => tab.id === thirdId)?.title).toBe("SSH server222 (2)"); + expect(queryStore.tabs.find((tab) => tab.id === fourthId)?.title).toBe("SSH server222 (3)"); + + // Closing the highest suffix may reuse that now-free number, but must not + // collide with any session that remains open. + queryStore.tabs = queryStore.tabs.filter((tab) => tab.id !== fourthId); + const fifthId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { + title: "SSH server222", + connectionId: "conn-1", + context: { connectionId: "conn-1", workbenchId: "wb-5" }, + forceNew: true, + }); + expect(queryStore.tabs.find((tab) => tab.id === fifthId)?.title).toBe("SSH server222 (3)"); + }); + + it("numbers sessions independently per connection and reads bridge-style context-only connection ids", async () => { + const queryStore = useQueryStore(); + + queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { title: "A", connectionId: "conn-1", context: { connectionId: "conn-1" }, forceNew: true }); + const otherConnectionId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { title: "B", connectionId: "conn-2", context: { connectionId: "conn-2" }, forceNew: true }); + // A different connection keeps its own bare title, unaffected by conn-1's count. + expect(queryStore.tabs.find((tab) => tab.id === otherConnectionId)?.title).toBe("B"); + + // Bridge-path legacy tab: connection only inside context (tab-level ""). + // It must join conn-1's numbering (second conn-1 tab → (1)) and its own + // tab-level connectionId gets normalized for reconnect/restore logic. + const bridgedId = queryStore.openPluginWorkbench("io.dbx.ssh", "workbench", { title: "A", context: { connectionId: "conn-1" }, forceNew: true }); + const bridged = queryStore.tabs.find((tab) => tab.id === bridgedId); + expect(bridged?.title).toBe("A (1)"); + expect(bridged?.connectionId).toBe("conn-1"); + }); }); diff --git a/apps/desktop/src/stores/connectionStore.ts b/apps/desktop/src/stores/connectionStore.ts index eeba9aa0fb..31614d4987 100644 --- a/apps/desktop/src/stores/connectionStore.ts +++ b/apps/desktop/src/stores/connectionStore.ts @@ -4334,8 +4334,8 @@ export const useConnectionStore = defineStore("connection", () => { invalidateConnectionMetadataLifetime(connectionId, database); } - async function ensureConnected(connectionId: string, options: { activate?: boolean; verifyHealth?: boolean } = {}) { - if (connectedIds.value.has(connectionId)) { + async function ensureConnected(connectionId: string, options: { activate?: boolean; verifyHealth?: boolean; forceReconnect?: boolean } = {}) { + if (!options.forceReconnect && connectedIds.value.has(connectionId)) { // Pure navigation can safely trust the existing connected state. Its // destination will perform the real API request, while blocking here on // a health probe makes an otherwise local tab switch take up to 5s. @@ -4450,6 +4450,49 @@ export const useConnectionStore = defineStore("connection", () => { } } + /** + * Re-push an already-open plugin connection's config (credentials included) + * to its sidecar through the same connection/connect path used when opening + * from the sidebar. A plugin iframe reload can leave the sidecar's in-memory + * connection registry empty while the host still considers the connection + * open, and ensureConnected()'s health fast-path would not heal that. Silent + * no-op when the connection is not open, is not plugin-backed, or its + * credentials are no longer available (save_password=false without a live + * session credential): the plugin then keeps its existing "reopen from the + * sidebar" guidance instead of triggering an interactive prompt from a + * background re-init. + */ + async function repushPluginConnection(connectionId: string): Promise { + const config = getConfig(connectionId); + if (!config || config.db_type !== "plugin" || !connectedIds.value.has(connectionId)) return; + // A successful connect/health probe within the TTL means the sidebar open + // (or a fresh restore connect) pushed the config moments ago and the + // sidecar registry cannot plausibly be empty yet — skipping here keeps the + // first open from paying a redundant disconnect+connect cycle on the + // plugin's first `ready`. The 2s in-memory TTL dies with the frontend, so + // every realistic reload path still re-pushes. + if (hasRecentConnectionHealthCheck(connectionId)) return; + if (connectionNeedsPasswordPrompt(config) && (await pluginConnectionPasswordPromptNeeded(config)) && !(await hasSessionCredential(connectionId))) return; + await ensureConnected(connectionId, { activate: false, forceReconnect: true }); + } + + /** + * Explicit, user-triggered reconnect of a plugin connection (the plugin's own + * "reconnect" button). Unlike repushPluginConnection — the silent background + * heal — this runs the full connect flow and MAY show the interactive + * password prompt, which is appropriate for a deliberate user action. Editing + * a connection drops it from connectedIds without notifying plugins, so the + * sidecar's config goes stale; this is the plugin's way to request a fresh + * connection/connect with the updated config. + */ + async function reopenPluginConnection(connectionId: string, pluginId: string): Promise { + const config = getConfig(connectionId); + if (!config) throw new Error("Connection config not found"); + if (config.db_type !== "plugin") throw new Error("Connection is not plugin-backed"); + if (config.plugin_id !== pluginId) throw new Error("Connection is owned by another plugin"); + await ensureConnected(connectionId, { activate: false, forceReconnect: true }); + } + function setBeforeConnectHandler(handler: BeforeConnectHandler | null) { beforeConnectHandler = handler; } @@ -9334,6 +9377,8 @@ export const useConnectionStore = defineStore("connection", () => { startCreatingConnectionInGroup, stopCreatingConnectionInGroup, connect, + repushPluginConnection, + reopenPluginConnection, cancelConnecting, disconnect, hasDisconnectInFlight, diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index 4b558f3e48..eea83ea366 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -3465,9 +3465,23 @@ export const useQueryStore = defineStore("query", () => { tab.nacosTargetKeyword = undefined; } + /** + * Effective connection binding of a plugin tab: bridge-created tabs carry + * the connection only inside context (tab-level connectionId is ""), so + * read both. New tabs normalize the context value up to the tab level at + * creation, but restored/legacy tabs still need the fallback. + */ + function pluginTabConnectionId(tab: QueryTab): string { + if (tab.connectionId) return tab.connectionId; + const contextConnectionId = tab.pluginWorkbench?.context?.connectionId; + return typeof contextConnectionId === "string" ? contextConnectionId : ""; + } + function openPluginWorkbench(pluginId: string, contributionId: string, options: { title?: string; connectionId?: string; database?: string; context?: Record; forceNew?: boolean } = {}) { + const contextConnectionId = typeof options.context?.connectionId === "string" ? options.context.connectionId : ""; + const connectionId = options.connectionId || contextConnectionId; if (!options.forceNew) { - const existing = tabs.value.find((tab) => tab.mode === "plugin-workbench" && tab.pluginWorkbench?.pluginId === pluginId && tab.pluginWorkbench?.contributionId === contributionId && tab.connectionId === (options.connectionId || "")); + const existing = tabs.value.find((tab) => tab.mode === "plugin-workbench" && tab.pluginWorkbench?.pluginId === pluginId && tab.pluginWorkbench?.contributionId === contributionId && pluginTabConnectionId(tab) === connectionId); if (existing) { // Reopening surfaces the existing tab as-is. Replacing the context // here (openPluginConnection mints a fresh workbenchId per click) @@ -3484,11 +3498,23 @@ export const useQueryStore = defineStore("query", () => { } } + // Termius-style session numbering: the first same-plugin + same-connection + // tab keeps the bare connection name; each additional one gets " (n)" + // where n advances beyond the highest live suffix (second tab → (1)). + // Numbers are assigned at creation and never backfilled after a close — + // stable titles beat dense numbering, while live titles stay unique. + const siblingTabs = tabs.value.filter((tab) => tab.mode === "plugin-workbench" && tab.pluginWorkbench?.pluginId === pluginId && pluginTabConnectionId(tab) === connectionId); + const nextSessionNumber = + siblingTabs.reduce((highest, tab) => { + const suffix = / \((\d+)\)$/.exec(tab.title); + return suffix ? Math.max(highest, Number(suffix[1])) : highest; + }, 0) + 1; + const baseTitle = options.title || contributionId; const id = uuid(); const tab: QueryTab = { id, - title: options.title || contributionId, - connectionId: options.connectionId || "", + title: siblingTabs.length >= 1 ? `${baseTitle} (${nextSessionNumber})` : baseTitle, + connectionId, database: options.database || "", sql: "", isExecuting: false, diff --git a/crates/dbx-core/src/connection/mod.rs b/crates/dbx-core/src/connection/mod.rs index e90e85ca17..e79d95522b 100644 --- a/crates/dbx-core/src/connection/mod.rs +++ b/crates/dbx-core/src/connection/mod.rs @@ -5038,6 +5038,21 @@ impl AppState { self.pool_routing_control().close_removed(removed).await; } + /// Drop a connection's pools WITHOUT closing them. Only for plugin + /// connections on the connect path: the plugin's connection/connect is an + /// idempotent upsert, so a re-push/reconnect just replaces the pool entry. + /// Closing would send connection/disconnect to the sidecar, whose + /// semantics are "drop the registry entry AND kill every session of this + /// connection" — murdering the live terminals of sibling tabs. Explicit + /// user disconnect still goes through remove_connection_pools* and does + /// send connection/disconnect. + pub async fn drop_connection_pools_without_close(&self, connection_id: &str) { + self.rollback_manual_transaction_sessions(connection_id).await; + let removed = self.drain_connection_pools(connection_id).await; + self.clear_metadata_gates_for_connection(connection_id).await; + drop(removed); + } + pub async fn remove_connection_pools_detached(&self, connection_id: &str) { self.rollback_manual_transaction_sessions(connection_id).await; let removed = self.drain_connection_pools(connection_id).await; diff --git a/crates/dbx-plugin-runtime/src/plugins/host.rs b/crates/dbx-plugin-runtime/src/plugins/host.rs index ee341b718e..2c2f37a265 100644 --- a/crates/dbx-plugin-runtime/src/plugins/host.rs +++ b/crates/dbx-plugin-runtime/src/plugins/host.rs @@ -193,7 +193,6 @@ impl PluginHost { let (_, provider) = self.resolve_connection_provider(config)?; plugin_connection_params(config, &provider, &config.host, config.port) } - pub async fn test_connection( &self, config: &ConnectionConfig, @@ -217,7 +216,7 @@ impl PluginHost { PLUGIN_CONNECTION_TEST_METHOD, plugin_connection_params(config, &provider, runtime_host, runtime_port)?, None, - Some(Duration::from_secs(config.effective_connect_timeout_secs())), + Some(plugin_connect_deadline(config, &provider)), ) .await?; plugin_connection_test_result(result, provider_label) @@ -251,7 +250,7 @@ impl PluginHost { PLUGIN_CONNECTION_CONNECT_METHOD, params.clone(), None, - Some(Duration::from_secs(config.effective_connect_timeout_secs())), + Some(plugin_connect_deadline(config, &provider)), ) .await?; ensure_plugin_operation_succeeded(result)?; @@ -406,6 +405,29 @@ fn required_plugin_binding<'a>(value: &'a Option, field: &str) -> Result .ok_or_else(|| format!("Plugin connection is missing {field}")) } +/// RPC deadline for connection/test and connection/connect. A config-bound +/// `connect_timeout_secs` field is the plugin's own handshake timeout (the SSH +/// plugin defaults it to 30 and lets advanced users tune it), so the host +/// deadline must never fire first: the resolved plugin value wins — stored +/// `external_config` first, then the manifest default for configs whose value +/// was never materialized (imports, MCP, older hosts) — and only plugins that +/// do not declare the field at all keep the generic built-in fallback. +fn plugin_connect_deadline(config: &ConnectionConfig, provider: &PluginConnectionProviderContribution) -> Duration { + fn positive_timeout(value: &serde_json::Value) -> Option { + value.as_u64().or_else(|| value.as_f64().map(|n| n.max(0.0) as u64)).filter(|secs| *secs > 0) + } + let plugin_timeout = provider.fields.iter().find(|field| field.key == "connect_timeout_secs").and_then(|field| { + config + .external_config + .as_ref() + .and_then(|external| external.get("connect_timeout_secs")) + .and_then(positive_timeout) + .or_else(|| field.default.as_ref().and_then(positive_timeout)) + }); + let secs = plugin_timeout.unwrap_or_else(|| config.effective_connect_timeout_secs()); + Duration::from_secs(secs.clamp(1, 300)) +} + fn plugin_connection_params( config: &ConnectionConfig, provider: &PluginConnectionProviderContribution, @@ -753,8 +775,9 @@ fn ensure_permission(plugin: &super::InstalledPlugin, required_permission: Optio #[cfg(test)] mod tests { use super::{ - plugin_connection_action_result, plugin_field_is_visible, plugin_invoke_connection_action, - validate_plugin_connection_values, validate_plugin_connection_values_for_action, + plugin_connect_deadline, plugin_connection_action_result, plugin_field_is_visible, + plugin_invoke_connection_action, validate_plugin_connection_values, + validate_plugin_connection_values_for_action, }; use crate::models::connection::ConnectionConfig; use crate::plugins::PluginConnectionProviderContribution; @@ -810,6 +833,58 @@ mod tests { assert!(lifecycle.begin_update("sample.ui").is_ok()); } + #[test] + fn connect_deadline_follows_the_provider_declared_timeout_field() { + let provider: PluginConnectionProviderContribution = serde_json::from_value(serde_json::json!({ + "id": "sample.connection", + "label": "Sample", + "database_type": "sample", + "fields": [{ "key": "connect_timeout_secs", "label": "Connect timeout", "type": "number", "default": 30 }] + })) + .unwrap(); + let config_with = |timeout: u64, external: Option| { + serde_json::from_value::(serde_json::json!({ + "id": "plugin-connection", + "name": "Plugin connection", + "db_type": "plugin", + "host": "localhost", + "port": 0, + "username": "", + "password": "", + "connect_timeout_secs": timeout, + "external_config": external.map_or(serde_json::json!({}), |secs| serde_json::json!({ "connect_timeout_secs": secs })), + "plugin_id": "sample", + "plugin_connection_provider": "sample.connection", + "plugin_connection_type": "sample" + })) + .unwrap() + }; + + // Unmaterialized typed value (0): the manifest default becomes the deadline. + assert_eq!(plugin_connect_deadline(&config_with(0, None), &provider), std::time::Duration::from_secs(30)); + // A stored plugin-field value wins over the typed field: the plugin's own + // handshake timeout and the host deadline must never disagree. + assert_eq!(plugin_connect_deadline(&config_with(10, Some(60)), &provider), std::time::Duration::from_secs(60)); + assert_eq!(plugin_connect_deadline(&config_with(5, None), &provider), std::time::Duration::from_secs(30)); + + // Providers without the well-known field keep the typed/generic behavior. + let provider_without_default: PluginConnectionProviderContribution = + serde_json::from_value(serde_json::json!({ + "id": "sample.connection", + "label": "Sample", + "database_type": "sample" + })) + .unwrap(); + assert_eq!( + plugin_connect_deadline(&config_with(0, Some(60)), &provider_without_default), + std::time::Duration::from_secs(10) + ); + assert_eq!( + plugin_connect_deadline(&config_with(5, None), &provider_without_default), + std::time::Duration::from_secs(5) + ); + } + #[test] fn rejects_zero_port_for_port_bound_connection_field() { let config: ConnectionConfig = serde_json::from_value(serde_json::json!({ diff --git a/plugins/README.md b/plugins/README.md index 16e31c312c..d39889c520 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -224,6 +224,8 @@ Password fields default to `secret` when `binding` is omitted. DBX validates req Absent optional fields stay absent: when DBX hands the manifest to its own UI it omits `description`, `placeholder`, `default`, and `binding` for fields that do not declare them, and `"default": null` means "no default" exactly like omitting the key. Treat a missing value as unset — never as an empty string, and never as the literal text `null`, which is not a storable plugin value. +Well-known field keys: a `config`-bound field keyed `connect_timeout_secs` declares the plugin's own connect/handshake timeout and is the single source of truth for it. On save, DBX mirrors its resolved value (the declared `default`, or the value a user entered in the connection form) into the typed `ConnectionConfig.connect_timeout_secs` — the dialog's generic global/per-connection timeout radios do not apply to providers declaring this field. The host's `connection/test` and `connection/connect` RPC deadline follows the same resolved value (stored `external_config` first, then the declared `default`), so the deadline never fires before the plugin's own timeout; providers that do not declare the field keep the generic typed-timeout behavior. Declare it when your transport needs more than the generic built-in 10s default (e.g. SSH handshakes on slow links). + #### Local file fields A `text`, `password`, or `textarea` field may declare `picker` when the user should choose a local file (private keys, keystores, credential files): diff --git a/plugins/sdk/dev-host/browser-bridge.mjs b/plugins/sdk/dev-host/browser-bridge.mjs index fdd1edff64..fec76eec03 100644 --- a/plugins/sdk/dev-host/browser-bridge.mjs +++ b/plugins/sdk/dev-host/browser-bridge.mjs @@ -121,8 +121,9 @@ function installBridge(channel) { const asset = await request("ui.readAsset", { path }); return URL.createObjectURL(new Blob([decode(asset.dataBase64)], { type: asset.contentType })); }, - openWorkbench: (contributionId, context) => request("host.openWorkbench", { contributionId, context }), + openWorkbench: (contributionId, context, options) => request("host.openWorkbench", { contributionId, context, forceNew: !!(options && options.forceNew) }), openFilesystem: (providerId, context) => request("host.openFilesystem", { providerId, context }), + reopenConnection: (connectionId) => request("host.reopenConnection", { connectionId }), copy: (text) => request("host.copy", { text }), onContext: (fn) => listen("context", fn), onEvent: (fn) => listen("event", fn), diff --git a/plugins/sdk/dev-host/package.json b/plugins/sdk/dev-host/package.json index 35a44526ae..7b9c5fc524 100644 --- a/plugins/sdk/dev-host/package.json +++ b/plugins/sdk/dev-host/package.json @@ -4,7 +4,7 @@ "type": "module", "scripts": { "build": "node build.mjs", - "test": "node --test tests/*.test.mjs", + "test": "node --test --test-timeout=30000 tests/*.test.mjs", "lint": "oxlint --vue-plugin --deny-warnings *.mjs ui tests", "fmt": "oxfmt \"*.mjs\" \"ui/**/*.{js,vue,css,html}\" \"tests/**/*.mjs\"", "fmt:check": "oxfmt --check \"*.mjs\" \"ui/**/*.{js,vue,css,html}\" \"tests/**/*.mjs\"" diff --git a/plugins/sdk/dev-host/process-tree.mjs b/plugins/sdk/dev-host/process-tree.mjs index 5302961b38..497a835113 100644 --- a/plugins/sdk/dev-host/process-tree.mjs +++ b/plugins/sdk/dev-host/process-tree.mjs @@ -5,11 +5,25 @@ import { setTimeout as delay } from "node:timers/promises"; export async function stopProcessTree(child) { if (!child?.pid) return; if (process.platform === "win32") { - await new Promise((resolve) => { - const killer = spawn("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" }); - killer.once("error", resolve); - killer.once("close", resolve); + if (child.exitCode !== null || child.signalCode !== null) return; + let onExit; + const exited = new Promise((resolve) => { + onExit = resolve; + child.once("exit", onExit); }); + try { + await new Promise((resolve, reject) => { + const killer = spawn("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" }); + killer.once("error", reject); + killer.once("close", (code) => { + if (code === 0 || child.exitCode !== null || child.signalCode !== null) resolve(); + else reject(new Error(`taskkill failed with exit code ${code}`)); + }); + }); + await exited; + } finally { + child.removeListener("exit", onExit); + } return; } const signal = (value) => { diff --git a/plugins/sdk/dev-host/server.mjs b/plugins/sdk/dev-host/server.mjs index c9fc61e380..e76ee363f8 100644 --- a/plugins/sdk/dev-host/server.mjs +++ b/plugins/sdk/dev-host/server.mjs @@ -241,6 +241,10 @@ export async function createMockHost(options) { requirePermission(manifest, "host.workbench"); return { mockHostOpenFrame: await serialize(() => openFrame(session, p.context?.connectionId || frame.connectionId, p.contributionId, p.context)) }; } + case "host.reopenConnection": { + if (typeof p.connectionId !== "string" || !p.connectionId) throw new Error("connectionId is invalid"); + return { ok: true, mockReopenConnection: p.connectionId }; + } default: throw new Error(`Unsupported mock host method: ${input.method}`); } diff --git a/plugins/sdk/dev-host/tests/process-tree.test.mjs b/plugins/sdk/dev-host/tests/process-tree.test.mjs new file mode 100644 index 0000000000..32003b4dd6 --- /dev/null +++ b/plugins/sdk/dev-host/tests/process-tree.test.mjs @@ -0,0 +1,76 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import childProcess from "node:child_process"; +import { EventEmitter } from "node:events"; +import { syncBuiltinESMExports } from "node:module"; +import { setImmediate } from "node:timers/promises"; +import { stopProcessTree } from "../process-tree.mjs"; + +function windowsProcess(context) { + const platform = Object.getOwnPropertyDescriptor(process, "platform"); + const child = Object.assign(new EventEmitter(), { pid: 12345, exitCode: null, signalCode: null }); + const killer = new EventEmitter(); + const spawn = context.mock.method(childProcess, "spawn", () => killer); + syncBuiltinESMExports(); + Object.defineProperty(process, "platform", { ...platform, value: "win32" }); + context.after(() => { + child.emit("exit", 0); + killer.emit("close", 0); + Object.defineProperty(process, "platform", platform); + context.mock.restoreAll(); + syncBuiltinESMExports(); + }); + return { child, killer, spawn }; +} + +test("Windows shutdown waits for the child exit event after taskkill completes", async (context) => { + const { child, killer } = windowsProcess(context); + let completed = false; + const stopping = stopProcessTree(child).then(() => { + completed = true; + }); + killer.emit("close", 0); + await setImmediate(); + assert.equal(completed, false); + child.exitCode = 0; + child.emit("exit", 0); + await stopping; + assert.equal(child.listenerCount("exit"), 0); +}); + +test("Windows shutdown avoids taskkill for a child whose exit was already observed", async (context) => { + const { child, killer, spawn } = windowsProcess(context); + child.exitCode = 0; + const stopping = stopProcessTree(child); + killer.emit("close", 128); + await stopping; + assert.equal(spawn.mock.callCount(), 0); +}); + +test("Windows shutdown reports a taskkill spawn failure without retaining an exit listener", async (context) => { + const { child, killer } = windowsProcess(context); + const stopping = stopProcessTree(child); + const rejected = assert.rejects(stopping, /taskkill unavailable/); + killer.emit("error", new Error("taskkill unavailable")); + await rejected; + assert.equal(child.listenerCount("exit"), 0); +}); + +test("Windows shutdown reports a failed taskkill while the child remains alive", async (context) => { + const { child, killer } = windowsProcess(context); + const stopping = stopProcessTree(child); + const rejected = assert.rejects(stopping, /taskkill.*1/); + killer.emit("close", 1); + await rejected; + assert.equal(child.listenerCount("exit"), 0); +}); + +test("Windows shutdown handles a child exiting before taskkill closes", async (context) => { + const { child, killer } = windowsProcess(context); + const stopping = stopProcessTree(child); + child.exitCode = 0; + child.emit("exit", 0); + killer.emit("close", 128); + await stopping; + assert.equal(child.listenerCount("exit"), 0); +}); diff --git a/plugins/sdk/dev-host/tests/runtime.test.mjs b/plugins/sdk/dev-host/tests/runtime.test.mjs index f7247ed8c7..4e2be60912 100644 --- a/plugins/sdk/dev-host/tests/runtime.test.mjs +++ b/plugins/sdk/dev-host/tests/runtime.test.mjs @@ -51,10 +51,11 @@ test("development credentials cannot be placed inside UI resources", async (t) = }); test("closing development host terminates its UI watcher and releases its port", async (t) => { + let host; + t.after(() => host?.close()); const options = await project(t), pidFile = join(options.project, "watcher.pid"); - const host = await startDevelopment({ ...options, commands: { watch: [process.execPath, "-e", 'require("node:fs").writeFileSync(process.argv[1], String(process.pid)); setInterval(() => {}, 1000)', pidFile] } }); - t.after(() => host.close()); + host = await startDevelopment({ ...options, commands: { watch: [process.execPath, "-e", 'require("node:fs").writeFileSync(process.argv[1], String(process.pid)); setInterval(() => {}, 1000)', pidFile] } }); let pid; for (let attempt = 0; attempt < 100; attempt++) { try { @@ -71,9 +72,10 @@ test("closing development host terminates its UI watcher and releases its port", }); test("watcher success markers trigger reload only after a complete line", async (t) => { + let host; + t.after(() => host?.close()); const options = await project(t); - const host = await startDevelopment({ ...options, commands: { watch: [process.execPath, "-e", 'process.stdout.write("x".repeat(1100)+"DBX_UI_BUILD_SUCCESS\\n"); process.stdout.write("DBX_UI_BUILD_"); setTimeout(() => process.stdout.write("SUCCESS\\n"), 100); setInterval(() => {}, 1000)'] } }); - t.after(() => host.close()); + host = await startDevelopment({ ...options, commands: { watch: [process.execPath, "-e", 'process.stdout.write("x".repeat(1100)+"DBX_UI_BUILD_SUCCESS\\n"); process.stdout.write("DBX_UI_BUILD_"); setTimeout(() => process.stdout.write("SUCCESS\\n"), 100); setInterval(() => {}, 1000)'] } }); let revision = 0; for (let i = 0; i < 100 && !revision; i++) { await delay(20); diff --git a/plugins/sdk/dev-host/tests/server.test.mjs b/plugins/sdk/dev-host/tests/server.test.mjs index 994c60aee7..6b7d906520 100644 --- a/plugins/sdk/dev-host/tests/server.test.mjs +++ b/plugins/sdk/dev-host/tests/server.test.mjs @@ -264,10 +264,12 @@ test("source watching is opt-in and automatic restarts preserve saved connection }); test("auto-reload preference persists across dev host restarts", async (t) => { + let second; + t.after(() => second?.close()); const first = await fixture(t); assert.equal((await first.request("auto-reload", { enabled: true })).value.enabled, true); await first.host.close(); - const second = await createMockHost({ + second = await createMockHost({ project: first.root, uiRoot: "ui", backend: process.execPath, @@ -279,10 +281,6 @@ test("auto-reload preference persists across dev host restarts", async (t) => { backendWatch: first.root, buildBackend: async () => {}, }); - t.after(async () => { - await second.close(); - await rm(first.root, { recursive: true, force: true }); - }); const bootstrap = await (await fetch(`${second.origin}/api/bootstrap`)).json(); assert.equal(bootstrap.autoReload, true); }); diff --git a/plugins/sdk/dev-host/tests/sidecar.test.mjs b/plugins/sdk/dev-host/tests/sidecar.test.mjs index 2cec6d5626..3d8580a259 100644 --- a/plugins/sdk/dev-host/tests/sidecar.test.mjs +++ b/plugins/sdk/dev-host/tests/sidecar.test.mjs @@ -92,13 +92,16 @@ test("handshake rejects wrong identity and missing executables terminate promptl }); test("handshake adopts a manifest.json rewritten on disk when the in-memory copy is stale", async (t) => { + let sidecar; const project = await mkdtemp(join(tmpdir(), "dbx-sidecar-manifest-")); - t.after(() => rm(project, { recursive: true, force: true })); + t.after(async () => { + await sidecar?.stop(); + await rm(project, { recursive: true, force: true }); + }); // Simulates a version bump made while the dev host was already running: // the sidecar binary reports 1.0.0, the in-memory manifest is behind. await writeFile(join(project, "manifest.json"), JSON.stringify({ id: "example.echo", version: "1.0.0" })); - const sidecar = new Sidecar({ ...config, cwd: project, manifest: { id: "example.echo", version: "0.9.0" } }); - t.after(() => sidecar.stop()); + sidecar = new Sidecar({ ...config, cwd: project, manifest: { id: "example.echo", version: "0.9.0" } }); const info = await sidecar.start(); assert.equal(sidecar.state, "ready"); assert.equal(sidecar.manifest.version, "1.0.0"); @@ -106,10 +109,14 @@ test("handshake adopts a manifest.json rewritten on disk when the in-memory copy }); test("handshake still rejects when the on-disk manifest also disagrees with the sidecar", async (t) => { + let sidecar; const project = await mkdtemp(join(tmpdir(), "dbx-sidecar-manifest-")); - t.after(() => rm(project, { recursive: true, force: true })); + t.after(async () => { + await sidecar?.stop(); + await rm(project, { recursive: true, force: true }); + }); await writeFile(join(project, "manifest.json"), JSON.stringify({ id: "example.echo", version: "0.9.0" })); - const sidecar = new Sidecar({ ...config, cwd: project, manifest: { id: "example.echo", version: "0.9.0" } }); + sidecar = new Sidecar({ ...config, cwd: project, manifest: { id: "example.echo", version: "0.9.0" } }); await assert.rejects(sidecar.start(), /identity/); }); diff --git a/src-tauri/src/commands/connection.rs b/src-tauri/src/commands/connection.rs index d1fdf57cd6..087c332c4a 100644 --- a/src-tauri/src/commands/connection.rs +++ b/src-tauri/src/commands/connection.rs @@ -1687,7 +1687,16 @@ pub async fn connect_db( let mut connected_config = config.clone(); let mut connected_db_config = db_config.clone(); - state.remove_connection_pools_detached(&id).await; + // Plugin 连接的 connection/connect 是幂等 upsert:连接路径(首连、重推、 + // 重连)只替换池条目,完全跳过旧池拆除——close 会向 sidecar 发 + // connection/disconnect,其语义是"删注册表 + 杀掉该连接全部会话",会把 + // 同连接其他 tab 的活跃终端一起杀死。用户显式断开仍走 disconnect_db 的 + // 完整关闭路径(会发 disconnect)。其他类型维持 detached 关闭不变。 + if db_config.db_type == DatabaseType::Plugin { + state.drop_connection_pools_without_close(&id).await; + } else { + state.remove_connection_pools_detached(&id).await; + } drop_nacos_adapters_for_connection_ids(state.inner(), std::slice::from_ref(&id)).await; state.reset_connection_transport_for_config(&id, &db_config).await;