Skip to content

fix(connection-form): show the right default socket path per database type (#1902)#1904

Merged
datlechin merged 1 commit into
mainfrom
fix/socket-path-per-database-type
Jul 18, 2026
Merged

fix(connection-form): show the right default socket path per database type (#1902)#1904
datlechin merged 1 commit into
mainfrom
fix/socket-path-per-database-type

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #1902.

Problem

In the New/Edit Connection form, when SSH tunneling is enabled the Socket Path field showed the same PostgreSQL hint (/var/run/postgresql/.s.PGSQL.5432) for every database type, so MySQL and MariaDB connections got the wrong default.

Root cause

GeneralPaneView.swift hardcoded the field's placeholder to the PostgreSQL socket regardless of type. It is hint text only, never a saved value, so no stored connections were affected.

Fix

Route the placeholder through the existing per-type metadata system instead of hardcoding it, so it is correct for every driver. Same plumbing as defaultPort and hidesBuiltInPassword, entirely app-side: PluginMetadataSnapshot.ConnectionConfig is an internal app struct, not a PluginKit transfer type, so there is no PluginKit ABI bump and no plugin re-release.

  • Added defaultUnixSocketPath to ConnectionConfig.
  • Seeded MySQL / MariaDB → /var/run/mysqld/mysqld.sock, PostgreSQL → /var/run/postgresql/.s.PGSQL.5432, Redis → /var/run/redis/redis.sock.
  • Preserved the value in buildMetadataSnapshot so a loading plugin binary can't drop it (same pattern as hidesBuiltInPassword).
  • Added PluginManager.defaultUnixSocketPath(for:) and a NetworkPaneViewModel.socketPathPrompt computed property that falls back to a generic /path/to/database.sock for types with no socket convention.
  • Bound the field's prompt to socketPathPrompt.

The values match TablePro's own docs table in docs/databases/ssh-tunneling.mdx, so no docs change was needed.

Tests

  • SocketPathPlaceholderTestsdefaultUnixSocketPath(for:) returns the right path for MySQL, MariaDB, PostgreSQL, and Redis, and nil for SQLite, ClickHouse, and unknown types.
  • Extended NetworkPaneSocketForwardTestssocketPathPrompt follows the database type and falls back to the generic hint for a socketless type.

Notes

  • No new UI test: the sibling per-type placeholders (filePathPrompt, containerEntityPlaceholder) are covered at the metadata/view-model layer, not with UI automation, and this follows the same pattern.

@datlechin
datlechin merged commit 1815e1d into main Jul 18, 2026
2 of 3 checks passed
@datlechin
datlechin deleted the fix/socket-path-per-database-type branch July 18, 2026 04:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc44a6a0b5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +401 to +402
PluginMetadataRegistry.shared.snapshot(forTypeId: databaseType.pluginTypeId)?
.connection.defaultUnixSocketPath

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the concrete type for socket path metadata

When the connection form is opened for a PostgreSQL-backed alias such as Redshift or CockroachDB, databaseType.pluginTypeId resolves to PostgreSQL via the registry aliases, so this lookup returns /var/run/postgresql/.s.PGSQL.5432 even though those concrete snapshots do not define a Unix-socket convention. That leaves the new per-type placeholder wrong for those database types; this UI metadata should be read from the concrete type snapshot instead of the driver lookup ID.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wrong socket path

1 participant