diff --git a/.github/workflows/dprint-update.yml b/.github/workflows/dprint-update.yml index 8a9b283d..b335cc88 100644 --- a/.github/workflows/dprint-update.yml +++ b/.github/workflows/dprint-update.yml @@ -5,7 +5,10 @@ # # Implementation: # - Mondays 09:00 UTC, run `npx dprint config update -y` which bumps -# every plugin in dprint.json to its latest version. +# every plugin in dprint.json to its newest version that is at least 7 +# days old. dprint reads `min-release-age=7` from `.npmrc`, the same +# cooldown npm installs follow, so a bump can stop short of the latest +# release or skip a plugin; the step's log names each one held back (#958). # - Apply any formatting changes from new plugin rules via `npx dprint fmt`. # - If anything changed, open a PR via peter-evans/create-pull-request. # - If nothing changed, exit clean (no PR, no spam). @@ -52,8 +55,8 @@ jobs: - name: Install dependencies run: npm ci - # Bump every plugin in dprint.json to current latest. - # Output: deterministic diff to dprint.json. -y = non-interactive. + # Bump every plugin in dprint.json to its newest version older than + # the 7-day cooldown in `.npmrc`. -y = non-interactive. - name: Bump dprint plugins run: npx dprint config update -y @@ -70,7 +73,7 @@ jobs: run: | if [ -z "$(git status --porcelain)" ]; then echo "changed=false" >> "$GITHUB_OUTPUT" - echo "Every dprint plugin is already at its latest version." + echo "No dprint plugin has a newer version that is past the 7-day cooldown." else echo "changed=true" >> "$GITHUB_OUTPUT" echo "Files changed:" @@ -111,6 +114,11 @@ jobs: See #236 for the rationale and the schedule. + Each plugin moves to its newest version that is at least 7 days + old (`min-release-age` in `.npmrc`), so a plugin may stop short + of its latest release or not move at all. The rest follow in a + later run. + Auto-generated by `.github/workflows/dprint-update.yml`. **No checks will appear on this PR.** GitHub does not run diff --git a/CLAUDE.md b/CLAUDE.md index d292be76..1bb2e8ab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,6 +14,10 @@ without asking again. - **"Release" means ship it, end to end.** Use the `release` skill. Don't stop to ask which version or whether to go ahead. +- **Merge your own PRs into `dev`.** Once a PR you opened is green and + mergeable, squash-merge it without asking. The release PR from `dev` into + `main` is different: you merge it too, but only after the maintainer has + approved that release. Approval covers one release, not the next. - **Write lasting instructions down.** When the maintainer sets a rule for how to work, add it to `AGENTS.md` (any agent) or this file (Claude only) in the same session. Put a procedure in a skill instead. diff --git a/docs/RELEASING.md b/docs/RELEASING.md index bbad7011..6df08968 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -71,8 +71,11 @@ These are settings, not files, so a PR cannot make them: merges (for everything into `dev`). - **Actions may create pull requests** (Settings → Actions → General). The dprint bump already needs this. -- **If `main` has a branch ruleset, let GitHub Actions bypass it.** The bump - commit and the tag are pushed to `main` with `GITHUB_TOKEN`. +- **Let GitHub Actions bypass any ruleset or branch protection on `main` and + `dev`, and any tag protection matching `v*`.** `auto-release.yml` pushes + with `GITHUB_TOKEN`: the `bump` job pushes the bump commit and the `vX.Y.Z` + tag to `main`, and the `sync-dev` job pushes the merge of `main` back into + `dev`. Without a bypass, that job fails at its push. ### Pushes made with `GITHUB_TOKEN` start no workflows diff --git a/dprint.json b/dprint.json index d7bc6d7a..6e410aff 100644 --- a/dprint.json +++ b/dprint.json @@ -16,7 +16,7 @@ "plugins": [ "npm:@dprint/typescript@0.96.1", "npm:@dprint/json@0.25.2", - "npm:@dprint/markdown@0.26.0", + "npm:@dprint/markdown@0.28.2", "npm:dprint-plugin-yaml@0.6.0", "npm:@dprint/toml@0.9.0" ] diff --git a/src/components/editor/SQLEditor.tsx b/src/components/editor/SQLEditor.tsx index ad253536..86cb511c 100644 --- a/src/components/editor/SQLEditor.tsx +++ b/src/components/editor/SQLEditor.tsx @@ -4,7 +4,7 @@ import { useCallback, useEffect, useLayoutEffect, useRef } from "react"; import { useSchemaCompletion } from "../../hooks/useSchemaCompletion"; import { statementRange } from "../../lib/error-marker"; import { monacoThemeData, monacoThemeName } from "../../lib/monaco-theme"; -import { runConnectionId } from "../../lib/run-connection"; +import { liveConnectionIds, runConnectionId } from "../../lib/run-connection"; import { createCompletionProvider } from "../../lib/schema-completion-provider"; import { formatSql } from "../../lib/sql-format"; import { sqlAtCursor } from "../../lib/statement-at-cursor"; @@ -13,7 +13,6 @@ import { setPendingContentFlush, useEditorStore } from "../../stores/editorStore import { useResultStore } from "../../stores/resultStore"; import { useSchemaStore } from "../../stores/schemaStore"; import { useSettingsStore } from "../../stores/settingsStore"; -import { useSqliteStore } from "../../stores/sqliteStore"; import { useThemeStore } from "../../stores/themeStore"; /** @@ -27,14 +26,10 @@ import { useThemeStore } from "../../stores/themeStore"; function activeTabContext(): { connectionId?: string; database?: string; session?: string } { const { tabs, activeTabId } = useEditorStore.getState(); const tab = tabs.find((t) => t.id === activeTabId); - const { selectedConnectionId, activeConnections } = useConnectionStore.getState(); - const live = [ - ...activeConnections.map((c) => c.id), - ...useSqliteStore.getState().sessions.map((s) => s.id), - ]; + const { selectedConnectionId } = useConnectionStore.getState(); return { // The server the tab is marked with, not whichever is selected (#788). - connectionId: runConnectionId(tab, selectedConnectionId, live), + connectionId: runConnectionId(tab, selectedConnectionId, liveConnectionIds()), database: tab?.database, session: tab?.type === "query" ? tab.id : undefined, }; diff --git a/src/components/layout/__tests__/useConnectionSwitching.test.ts b/src/components/layout/__tests__/useConnectionSwitching.test.ts index 99381478..4baecf89 100644 --- a/src/components/layout/__tests__/useConnectionSwitching.test.ts +++ b/src/components/layout/__tests__/useConnectionSwitching.test.ts @@ -110,4 +110,29 @@ describe("editor tab and selected connection agree (#788)", () => { expect(activeTab()?.connectionId).toBe("sqlite-1"); expect(useEditorStore.getState().tabs.find((t) => t.id === "tab-0")?.connectionId).toBe("dev"); }); + + it("drops the old server's profile from a tab a SQLite file takes over (#962)", () => { + // The tab's server is gone, so the file takes the tab rather than a new one. + setConnections([], null); + const { result } = renderHook(() => useConnectionSwitching()); + useSqliteStore.setState({ sessions: [{ id: "sqlite-1", path: "data.db" } as unknown as SqliteSession] }); + + act(() => result.current.attachSqliteSession("sqlite-1")); + + expect(activeTab()?.id).toBe("tab-0"); + expect(activeTab()?.connectionId).toBe("sqlite-1"); + // Kept, it would reconnect p-dev and bind the tab back to it at the next start. + expect(activeTab()?.profileId).toBeUndefined(); + }); + + it("replaces the old server's profile on a tab a new server takes over", () => { + setConnections([], null); + const { result } = renderHook(() => useConnectionSwitching()); + setConnections([prod], "prod"); + + act(() => result.current.attachNewConnection(prod)); + + expect(activeTab()?.id).toBe("tab-0"); + expect(activeTab()?.profileId).toBe("p-prod"); + }); }); diff --git a/src/components/layout/useConnectionSwitching.ts b/src/components/layout/useConnectionSwitching.ts index 91e3c08d..7e8e9942 100644 --- a/src/components/layout/useConnectionSwitching.ts +++ b/src/components/layout/useConnectionSwitching.ts @@ -1,18 +1,10 @@ import { useEffect } from "react"; -import { runConnectionId } from "../../lib/run-connection"; +import { liveConnectionIds, runConnectionId } from "../../lib/run-connection"; import { useConnectionStore } from "../../stores/connectionStore"; import { useEditorStore } from "../../stores/editorStore"; import { useSqliteStore } from "../../stores/sqliteStore"; import type { ConnectionInfo } from "../../types"; -/** Every connection that can run a statement right now. */ -function liveConnectionIds(): string[] { - return [ - ...useConnectionStore.getState().activeConnections.map((c) => c.id), - ...useSqliteStore.getState().sessions.map((s) => s.id), - ]; -} - /** * Keep the selected connection and the active editor tab pointing at the * same server (#788). @@ -83,7 +75,10 @@ export function useConnectionSwitching() { if (active && !activeHasServer) { // Only a tab with no live server is taken over, so this does not // normally move a session; through the guard all the same (#843). - requestTabConnection(active.id, connectionId, conn?.database ?? undefined, conn?.profile_id); + // A SQLite file has no profile. `null` drops the one the tab still has + // from the server it was on, which would otherwise send it back there + // at the next start (#962). + requestTabConnection(active.id, connectionId, conn?.database ?? undefined, conn?.profile_id ?? null); return; } openTabFor(connectionId, conn); diff --git a/src/components/layout/useStartupReconnect.ts b/src/components/layout/useStartupReconnect.ts index 02c0a4af..ed5c8ad7 100644 --- a/src/components/layout/useStartupReconnect.ts +++ b/src/components/layout/useStartupReconnect.ts @@ -17,7 +17,7 @@ export function useStartupReconnect() { const { tabs, setTabConnection: setTabConn } = useEditorStore.getState(); const profileIds = [ ...new Set( - tabs.filter((t) => t.profileId).map((t) => t.profileId!), + tabs.flatMap((t) => (t.profileId ? [t.profileId] : [])), ), ]; diff --git a/src/components/schema/SchemaTree.tsx b/src/components/schema/SchemaTree.tsx index bfec03ab..3708c593 100644 --- a/src/components/schema/SchemaTree.tsx +++ b/src/components/schema/SchemaTree.tsx @@ -129,7 +129,9 @@ export function SchemaTree({ connectionId }: { connectionId: string }) { const selectDatabase = (dbName: string) => { if (!activeTabId) return; const conn = activeConnections.find((c) => c.id === connectionId); - requestTabConnection(activeTabId, connectionId, dbName, conn?.profile_id); + // `null` for a tree with no profile (a SQLite file), so the tab does not + // keep the profile of the server it is moved from (#962). + requestTabConnection(activeTabId, connectionId, dbName, conn?.profile_id ?? null); }; // Tracks pending single-click timers keyed by "db.name" diff --git a/src/hooks/__tests__/useLiveConnectionIds.test.ts b/src/hooks/__tests__/useLiveConnectionIds.test.ts new file mode 100644 index 00000000..8d246334 --- /dev/null +++ b/src/hooks/__tests__/useLiveConnectionIds.test.ts @@ -0,0 +1,51 @@ +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { liveConnectionIds } from "../../lib/run-connection"; +import { useConnectionStore } from "../../stores/connectionStore"; +import { type SqliteSession, useSqliteStore } from "../../stores/sqliteStore"; +import type { ConnectionInfo } from "../../types"; +import { useLiveConnectionIds } from "../useLiveConnectionIds"; + +vi.mock("@tauri-apps/api/core", () => ({ invoke: vi.fn() })); + +const server = (id: string) => ({ id }) as unknown as ConnectionInfo; +const file = (id: string) => ({ id }) as unknown as SqliteSession; + +beforeEach(() => { + useConnectionStore.setState({ activeConnections: [] }); + useSqliteStore.setState({ sessions: [] }); +}); + +describe("useLiveConnectionIds", () => { + it("lists connected servers and open SQLite files", () => { + useConnectionStore.setState({ activeConnections: [server("dev")] }); + useSqliteStore.setState({ sessions: [file("sqlite-1")] }); + + const { result } = renderHook(() => useLiveConnectionIds()); + + expect(result.current).toEqual(["dev", "sqlite-1"]); + }); + + it("follows servers and files as they come and go", () => { + const { result } = renderHook(() => useLiveConnectionIds()); + expect(result.current).toEqual([]); + + act(() => useConnectionStore.setState({ activeConnections: [server("dev"), server("prod")] })); + act(() => useSqliteStore.setState({ sessions: [file("sqlite-1")] })); + expect(result.current).toEqual(["dev", "prod", "sqlite-1"]); + + act(() => useConnectionStore.setState({ activeConnections: [server("prod")] })); + expect(result.current).toEqual(["prod", "sqlite-1"]); + }); + + // Where a run goes and whether a tab's connection still counts (#944) read + // one list each; they must not drift apart. + it("is the same list as liveConnectionIds", () => { + useConnectionStore.setState({ activeConnections: [server("dev")] }); + useSqliteStore.setState({ sessions: [file("sqlite-1"), file("sqlite-2")] }); + + const { result } = renderHook(() => useLiveConnectionIds()); + + expect(result.current).toEqual(liveConnectionIds()); + }); +}); diff --git a/src/hooks/useLiveConnectionIds.ts b/src/hooks/useLiveConnectionIds.ts new file mode 100644 index 00000000..ff9aa530 --- /dev/null +++ b/src/hooks/useLiveConnectionIds.ts @@ -0,0 +1,22 @@ +import { useMemo } from "react"; +import { useConnectionStore } from "../stores/connectionStore"; +import { useSqliteStore } from "../stores/sqliteStore"; + +/** + * Every connection that can run a statement right now, for a component: the + * connected servers and the open SQLite files. + * + * `liveConnectionIds` (lib/run-connection) is the same list read once, for + * code outside a component. This one re-renders its caller when a server + * connects or disconnects or a file opens or closes. Both have to stay the + * same list: where a run goes and whether a tab's connection still counts + * (#944) depend on it. + */ +export function useLiveConnectionIds(): string[] { + const activeConnections = useConnectionStore((s) => s.activeConnections); + const sqliteSessions = useSqliteStore((s) => s.sessions); + return useMemo( + () => [...activeConnections.map((c) => c.id), ...sqliteSessions.map((s) => s.id)], + [activeConnections, sqliteSessions], + ); +} diff --git a/src/hooks/useQueryExecution.ts b/src/hooks/useQueryExecution.ts index 07a82f9a..e151f607 100644 --- a/src/hooks/useQueryExecution.ts +++ b/src/hooks/useQueryExecution.ts @@ -3,7 +3,7 @@ import { runConnectionId } from "../lib/run-connection"; import { useConnectionStore } from "../stores/connectionStore"; import { useEditorStore } from "../stores/editorStore"; import { useResultStore } from "../stores/resultStore"; -import { useSqliteStore } from "../stores/sqliteStore"; +import { useLiveConnectionIds } from "./useLiveConnectionIds"; /** * The editor toolbar's execution context, pre-bound. @@ -27,8 +27,7 @@ export function useQueryExecution() { const selectedConnectionId = useConnectionStore( (s) => s.selectedConnectionId, ); - const activeConnections = useConnectionStore((s) => s.activeConnections); - const sqliteSessions = useSqliteStore((s) => s.sessions); + const liveConnectionIds = useLiveConnectionIds(); const activeTabId = useEditorStore((s) => s.activeTabId); const tabs = useEditorStore((s) => s.tabs); const activeTab = tabs.find((t) => t.id === activeTabId); @@ -41,10 +40,7 @@ export function useQueryExecution() { const isExecuting = useResultStore((s) => s.isExecuting); // The server the tab is marked with, not whichever is selected (#788). - const connectionId = runConnectionId(activeTab, selectedConnectionId, [ - ...activeConnections.map((c) => c.id), - ...sqliteSessions.map((s) => s.id), - ]); + const connectionId = runConnectionId(activeTab, selectedConnectionId, liveConnectionIds); const database = activeTab?.database; // The tab's own server session, so what one run sets up is there for the // next (#731). Only a query tab has one; the others are views of an object. diff --git a/src/hooks/useResultsFollowActiveTab.ts b/src/hooks/useResultsFollowActiveTab.ts index 35cc4ec8..4a8e7342 100644 --- a/src/hooks/useResultsFollowActiveTab.ts +++ b/src/hooks/useResultsFollowActiveTab.ts @@ -3,7 +3,7 @@ import { runConnectionId } from "../lib/run-connection"; import { useConnectionStore } from "../stores/connectionStore"; import { useEditorStore } from "../stores/editorStore"; import { useResultStore } from "../stores/resultStore"; -import { useSqliteStore } from "../stores/sqliteStore"; +import { useLiveConnectionIds } from "./useLiveConnectionIds"; /** * Keep a grid read on one server from being shown under another (#842). @@ -19,16 +19,12 @@ export function useResultsFollowActiveTab() { const activeTabId = useEditorStore((s) => s.activeTabId); const tabs = useEditorStore((s) => s.tabs); const selectedConnectionId = useConnectionStore((s) => s.selectedConnectionId); - const activeConnections = useConnectionStore((s) => s.activeConnections); - const sqliteSessions = useSqliteStore((s) => s.sessions); + const liveConnectionIds = useLiveConnectionIds(); const dropForeignResults = useResultStore((s) => s.dropForeignResults); const activeTab = tabs.find((t) => t.id === activeTabId); // The tab's own server, as Run would pick it (#788). - const tabConnectionId = runConnectionId(activeTab, selectedConnectionId, [ - ...activeConnections.map((c) => c.id), - ...sqliteSessions.map((s) => s.id), - ]); + const tabConnectionId = runConnectionId(activeTab, selectedConnectionId, liveConnectionIds); useEffect(() => { dropForeignResults(tabConnectionId); diff --git a/src/lib/run-connection.ts b/src/lib/run-connection.ts index b6277812..f143e43f 100644 --- a/src/lib/run-connection.ts +++ b/src/lib/run-connection.ts @@ -1,5 +1,23 @@ +import { useConnectionStore } from "../stores/connectionStore"; +import { useSqliteStore } from "../stores/sqliteStore"; import type { EditorTab } from "../types"; +/** + * Every connection that can run a statement right now: the connected servers + * and the open SQLite files. + * + * The one definition of "live" for code outside a component. Where a run goes + * (`runConnectionId`) and whether a tab's own connection still counts + * (`resultStore`, #944) have to agree on it, or a tab is left bound to a + * connection Run no longer uses. + */ +export function liveConnectionIds(): string[] { + return [ + ...useConnectionStore.getState().activeConnections.map((c) => c.id), + ...useSqliteStore.getState().sessions.map((s) => s.id), + ]; +} + /** * Which connection Run sends the active tab's statements to. * @@ -8,9 +26,9 @@ import type { EditorTab } from "../types"; * selected connection instead, so a tab marked as production could run on * dev and the reverse (#788). The selected connection is the answer only for a * tab that has no live connection of its own: a fresh tab, or one whose - * server has gone — and such a tab carries no server markers either. A fresh - * tab is bound to where its first run went (`resultStore`, #942), and from - * then on runs there like any other. + * server has gone — and such a tab carries no server markers either. Either + * is bound to where that run went (`resultStore`, #942, #944), and from then + * on runs there like any other. */ export function runConnectionId( tab: Pick | undefined, diff --git a/src/lib/transaction-tracking.ts b/src/lib/transaction-tracking.ts index db7443fe..9b58c04d 100644 --- a/src/lib/transaction-tracking.ts +++ b/src/lib/transaction-tracking.ts @@ -4,9 +4,10 @@ * * The backend reports the session's state on a run's last result (#916), or * on the error of a run that failed (#917), and the editor takes that when it - * is there. It is not always: MySQL has no `@@in_transaction` to read, and the - * server may not say. Then the state is read off the statements themselves. That - * is a best guess, and errs towards "open": the indicator and the prompts it + * is there. It asks MySQL and MariaDB alike, so it is missing only when the + * probe could not get an answer: the session could not be reached, or the + * probe failed or timed out. Then the state is read off the statements + * themselves. That is a best guess, and errs towards "open": the indicator and the prompts it * drives cost a click when wrong, while missing a transaction rolls back the * user's work without a word. */ diff --git a/src/stores/__tests__/editorStore.test.ts b/src/stores/__tests__/editorStore.test.ts index 4ae12642..668af283 100644 --- a/src/stores/__tests__/editorStore.test.ts +++ b/src/stores/__tests__/editorStore.test.ts @@ -522,6 +522,22 @@ describe("editorStore", () => { expect(tab?.profileId).toBeUndefined(); }); + it("keeps the profile it has when none is passed", () => { + const id = useEditorStore.getState().addTab(); + useEditorStore.getState().setTabConnection(id, "conn-1", undefined, "profile-1"); + useEditorStore.getState().setTabConnection(id, "conn-1", "other_db"); + expect(useEditorStore.getState().tabs.find((t) => t.id === id)?.profileId).toBe("profile-1"); + }); + + it("clears the profile when passed null, for a connection that has none (#944)", () => { + const id = useEditorStore.getState().addTab(); + useEditorStore.getState().setTabConnection(id, "conn-1", undefined, "profile-1"); + useEditorStore.getState().setTabConnection(id, "lite-1", undefined, null); + const tab = useEditorStore.getState().tabs.find((t) => t.id === id); + expect(tab?.connectionId).toBe("lite-1"); + expect(tab?.profileId).toBeUndefined(); + }); + it("should not affect other tabs", () => { const id1 = useEditorStore.getState().addTab(); const id2 = useEditorStore.getState().addTab(); diff --git a/src/stores/__tests__/resultStore.test.ts b/src/stores/__tests__/resultStore.test.ts index 774384dd..f2fd4aed 100644 --- a/src/stores/__tests__/resultStore.test.ts +++ b/src/stores/__tests__/resultStore.test.ts @@ -82,6 +82,7 @@ import type { EditorTab, QueryResult } from "../../types"; import { useEditorStore } from "../editorStore"; import { useProductionGuardStore } from "../productionGuardStore"; import { anyTabExecuting, followEditorStore, resetRunBookkeeping, useResultStore } from "../resultStore"; +import { useSqliteStore } from "../sqliteStore"; import { hasOpenTransactionForTab, useTransactionStore } from "../transactionStore"; // Module state outside the store: the grid's unsaved-edit counts (#865) and @@ -1378,7 +1379,8 @@ describe("resultStore", () => { expect(bound("tab-1")).toMatchObject({ connectionId: "conn-1", profileId: "p1" }); }); - it("leaves a tab that already has a connection where it is", async () => { + it("leaves a tab that already has a live connection where it is", async () => { + connectionStoreState.activeConnections.push({ id: "conn-2", profile_id: "p2", name: "Other DB" }); await useResultStore.getState().executeQuery("conn-1", "SELECT 1", "shop", "tab-2"); expect(bound("tab-2")).toMatchObject({ connectionId: "conn-2", database: "shop" }); @@ -1399,6 +1401,111 @@ describe("resultStore", () => { }); }); + describe("a tab whose connection is gone, on its next run (#944)", () => { + const tab = (id: string, connectionId?: string, profileId?: string): EditorTab => ({ + id, + title: id, + content: "", + type: "query", + isDirty: false, + connectionId, + profileId, + }); + const bound = (id: string) => useEditorStore.getState().tabs.find((t) => t.id === id); + const sqliteSession = (id: string) => ({ id, path: `/data/${id}.db`, name: `${id}.db`, openedAt: "" }); + const begin = () => + sqliteExecuteMock.mockResolvedValueOnce([ + { ...makeQueryResult({ sql: "BEGIN" }), in_transaction: true, truncation_reason: null }, + ]); + + beforeEach(() => { + registerConnectionKind("lite-1", "sqlite"); + useSqliteStore.setState({ sessions: [sqliteSession("lite-1")] }); + useTransactionStore.setState({ open: {}, openConnections: {} }); + // "gone-1" is in neither store: a server disconnected, or a file closed. + // A second query tab, so closing the first does not leave a fresh tab + // on its connection in its place. + useEditorStore.setState({ + tabs: [tab("tab-1", "gone-1", "p-gone"), tab("tab-2", "conn-1", "p1")], + activeTabId: "tab-1", + }); + }); + afterEach(() => { + forgetConnectionKind("lite-1"); + useSqliteStore.setState({ sessions: [] }); + }); + + it("is rebound to the server it ran on, with that server's profile", async () => { + await useResultStore.getState().executeQuery("conn-1", "SELECT 1", undefined, "tab-1"); + + expect(bound("tab-1")).toMatchObject({ connectionId: "conn-1", profileId: "p1" }); + }); + + it("is rebound to the SQLite file it ran on, and loses the old server's profile", async () => { + await useResultStore.getState().executeQuery("lite-1", "SELECT 1", undefined, "tab-1"); + + expect(bound("tab-1")?.connectionId).toBe("lite-1"); + // Kept, it would reconnect the tab to the old server at the next start. + expect(bound("tab-1")?.profileId).toBeUndefined(); + }); + + it("is rebound by a query plan too", async () => { + await useResultStore.getState().executeExplain("conn-1", "SELECT 1", undefined, undefined, "tab-1"); + + expect(bound("tab-1")).toMatchObject({ connectionId: "conn-1", profileId: "p1" }); + }); + + it("shows the transaction it opens on the file", async () => { + begin(); + await useResultStore.getState().executeQuery("lite-1", "BEGIN", undefined, "tab-1"); + + const t = bound("tab-1"); + expect(t && hasOpenTransactionForTab(t)).toBe(true); + }); + + it("has that transaction rolled back when it was the last tab on the file", async () => { + begin(); + await useResultStore.getState().executeQuery("lite-1", "BEGIN", undefined, "tab-1"); + sqliteExecuteMock.mockClear(); + sqliteExecuteMock.mockResolvedValueOnce([ + { ...makeQueryResult({ sql: "ROLLBACK" }), in_transaction: false, truncation_reason: null }, + ]); + + useEditorStore.getState().closeTab("tab-1"); + + expect(sqliteExecuteMock).toHaveBeenCalledWith("lite-1", "ROLLBACK"); + await vi.waitFor(() => expect(useTransactionStore.getState().openConnections).toEqual({})); + }); + + it("is asked about before it closes with that transaction open", async () => { + begin(); + await useResultStore.getState().executeQuery("lite-1", "BEGIN", undefined, "tab-1"); + + useEditorStore.getState().requestClose("close", "tab-1"); + + expect(useEditorStore.getState().pendingClose?.inTransaction.map((t) => t.id)).toEqual(["tab-1"]); + useEditorStore.getState().cancelPendingClose(); + }); + + it("stays where it is when its own connection is another live SQLite file", async () => { + useSqliteStore.setState({ sessions: [sqliteSession("lite-1"), sqliteSession("lite-2")] }); + useEditorStore.setState({ tabs: [tab("tab-1", "lite-2")] }); + + await useResultStore.getState().executeQuery("lite-1", "SELECT 1", undefined, "tab-1"); + + expect(bound("tab-1")?.connectionId).toBe("lite-2"); + }); + + it("stays where it is when its own connection is another live server", async () => { + connectionStoreState.activeConnections.push({ id: "conn-2", profile_id: "p2", name: "Other DB" }); + useEditorStore.setState({ tabs: [tab("tab-1", "conn-2", "p2")] }); + + await useResultStore.getState().executeQuery("lite-1", "SELECT 1", undefined, "tab-1"); + + expect(bound("tab-1")).toMatchObject({ connectionId: "conn-2", profileId: "p2" }); + }); + }); + describe("a script that fails part-way (#845)", () => { const SCRIPT = "SELECT * FROM products;\nSELECT * FROM nope;\nSELECT 1;"; diff --git a/src/stores/editorStore.ts b/src/stores/editorStore.ts index 85951128..a1f99d6a 100644 --- a/src/stores/editorStore.ts +++ b/src/stores/editorStore.ts @@ -155,14 +155,19 @@ interface EditorState { * server where its transaction is open (#843). `setTabConnection` sets it * unconditionally. */ - requestTabConnection: (id: string, connectionId: string, database?: string, profileId?: string) => void; + requestTabConnection: (id: string, connectionId: string, database?: string, profileId?: string | null) => void; setActiveTab: (id: string) => void; updateTabContent: (id: string, content: string) => void; + /** + * Point a tab at a connection. `profileId` is the saved profile to + * reconnect the tab by: omitted keeps the one it has, `null` clears it, for + * a connection that has none (a SQLite file). + */ setTabConnection: ( id: string, connectionId: string, database?: string, - profileId?: string, + profileId?: string | null, ) => void; setEditorInstance: (instance: editor.IStandaloneCodeEditor | null) => void; setTabDirty: (tabId: string, dirty: boolean) => void; @@ -372,7 +377,7 @@ export const useEditorStore = create((set, get) => ({ set((state) => ({ tabs: state.tabs.map((t) => { if (t.id !== id) return t; - const profile = profileId !== undefined ? { profileId } : {}; + const profile = profileId !== undefined ? { profileId: profileId ?? undefined } : {}; // A structure, designer or routine tab is *about* a database, so an // undefined one would leave it pointing at nothing. The flat interface // let that through; keep what the tab already has instead. Only a diff --git a/src/stores/resultStore.ts b/src/stores/resultStore.ts index 0daf8504..a6a2e8cf 100644 --- a/src/stores/resultStore.ts +++ b/src/stores/resultStore.ts @@ -3,6 +3,7 @@ import type { ExplainFormat, TransactionState } from "../lib/bindings"; import { connectionKind, dataSourceFor } from "../lib/datasource"; import { forgetGridMemory } from "../lib/grid-memory"; import { firstGridIndex } from "../lib/result-label"; +import { liveConnectionIds } from "../lib/run-connection"; import { noteSession } from "../lib/server-sessions"; import { splitSqlStatements } from "../lib/sql-import"; import { isDestructiveStatement } from "../lib/sql-safety"; @@ -901,7 +902,7 @@ async function doExplain( // Statement normalization (trailing `;`, multi-statement) and the // ANALYZE-safety decision both happen backend-side (#412, #418). if (session) { - bindUnboundTab(session, connectionId, connectionProfileId(connectionId)); + bindTabToRun(session, connectionId, connectionProfileId(connectionId)); noteSession(session, connectionId); } const response = await api.explainQuery(connectionId, sql, analyze, database, format, session); @@ -987,7 +988,7 @@ async function doExecuteQuery( // Noted before the call: the backend opens the session as the statement // starts, and a tab closed while it runs should still end it. const source = dataSourceFor(connectionId); - if (session) bindUnboundTab(session, connectionId, conn?.profile_id); + if (session) bindTabToRun(session, connectionId, conn?.profile_id); if (session && source.kind === "mysql") noteSession(session, connectionId); const results = await source.execute( connectionId, @@ -1134,34 +1135,47 @@ function connectionProfileId(connectionId: string): string | undefined { } /** - * Bind a tab that has no connection to the one its first run goes to (#942). + * Bind a tab with no live connection of its own to the one its run goes to + * (#942, #944). * - * A tab opened with "+" has none, and runs on the selected connection - * (`runConnectionId`). Once it has run there, it has state there — a MySQL - * session, or a SQLite file's open transaction — and everything that guards - * that state reads the tab's connection: the TX badge on a file's tabs, the - * prompt and rollback when the last tab on a file goes, the prompt before a - * tab is pointed at another server. An unbound tab slipped past all of them, - * and followed the selection to another server while its transaction stayed - * open on the first. Bound, it behaves as every tab opened on a connection - * does, and is marked with that connection's colour as Run already treats it. + * A tab opened with "+" has no connection, and one whose SQLite file was + * closed or whose server was disconnected still names a connection that is + * gone. Both run on the selected connection (`runConnectionId`). Once a tab + * has run there, it has state there — a MySQL session, or a SQLite file's + * open transaction — and everything that guards that state reads the tab's + * connection: the TX badge on a file's tabs, the prompt and rollback when the + * last tab on a file goes, the prompt before a tab is pointed at another + * server. A tab not bound to where it ran slipped past all of them, and + * followed the selection to another server while its transaction stayed open + * on the first. Bound, it behaves as every tab opened on a connection does, + * and is marked with that connection's colour as Run already treats it. + * + * Without asking, unlike `requestTabConnection`: the connection the tab is + * taken from is gone, and took with it anything there was to roll back. A tab + * on another connection that is still live is left where it is. */ -function bindUnboundTab(tabId: string, connectionId: string, profileId: string | undefined) { +function bindTabToRun(tabId: string, connectionId: string, profileId: string | undefined) { // A test that replaces the editor store with a bare mock has no tabs to bind. if (typeof useEditorStore.getState !== "function") return; const { tabs, setTabConnection } = useEditorStore.getState(); const tab = tabs?.find((t) => t.id === tabId); - if (!tab || tab.connectionId !== undefined || typeof setTabConnection !== "function") return; - setTabConnection(tabId, connectionId, tab.database, profileId); + if (!tab || tab.connectionId === connectionId || typeof setTabConnection !== "function") return; + if (tab.connectionId !== undefined && liveConnectionIds().includes(tab.connectionId)) return; + // The profile is replaced rather than kept: the old server's would send the + // tab back there at the next start (`useStartupReconnect`). A SQLite file + // has none, so there it is cleared. + setTabConnection(tabId, connectionId, tab.database, profileId ?? null); } /** * Record whether the tab's session is inside a transaction after a run (#843). * * The backend's answer wins: it asks the server after the run and reports it - * on the last result, or on the error of a run that failed (#917). It has - * none on MySQL (no `@@in_transaction`) or when the server could not say, so - * then the statements that ran are read instead (lib/transaction-tracking). + * on the last result, or on the error of a run that failed (#917). MySQL and + * MariaDB are both asked, by a probe that needs no `@@in_transaction`. It has + * no answer only when that probe could not get one — the session could not be + * reached, the probe failed or timed out — so then the statements that ran + * are read instead (lib/transaction-tracking). */ function noteTransaction(session: string, reported: TransactionState | null | undefined, ran: readonly string[]) { const open = reported ? reported === "open" : transactionAfter(ran, hasOpenTransaction(session));