fix(ssh): resolve saved connection by ID when testing with ID-only config

[dhunganapramod9]

Description

When the renderer calls sshTestConnection with only a connection ID and empty host/username (e.g. from SshConnectionTestButton), the main process previously used those empty values and the test could not succeed. This change detects that "ID-only" call shape, loads the saved connection from the DB, hydrates credentials from keytar, and runs the test against the resolved config.

Rationale

SshConnectionTestButton only has access to connectionId and was intentionally calling the IPC with ID + empty fields and a TODO for the main process to handle it. The main process did not handle that case; this fix adds the same resolve-by-ID pattern already used by the CONNECT handler.

Changes

• Main process (src/main/ipc/sshIpc.ts): In the TEST_CONNECTION handler, detect ID-only calls (config has id but host or username empty). For those, load the saved connection from the DB, hydrate password/passphrase from keytar, and run the test using the resolved config.
• Renderer (src/renderer/components/ssh/SshConnectionTestButton.tsx): Comment updated to state that the main process accepts ID-only and resolves from DB/keytar.

Testing

• No new tests; behavior aligns with the existing CONNECT handler (resolve by ID from DB + keytar).
• Manual: use "Test" on a saved SSH connection from the UI that uses SshConnectionTestButton.

Pre-PR Checklist

• Dev server runs: pnpm run d (or pnpm run dev) starts cleanly.
• Code is formatted: pnpm run format.
• Lint passes: pnpm run lint.
• Types check: pnpm run type-check.
• Tests pass: pnpm exec vitest run.
• No stray build artifacts or secrets committed.
• Documented any schema or config changes impacting users. (N/A — no schema/config changes.)

[vercel]

@dhunganapramod9 is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

This PR fixes the SSH "Test Connection" flow for saved connections: when SshConnectionTestButton calls the TEST_CONNECTION IPC channel with only a connectionId (and empty host/username), the main process now detects that "ID-only" call shape, loads the full connection record from the DB via Drizzle, hydrates credentials from keytar, and runs the actual test — matching the pattern already used by the CONNECT handler.

Key points:

• The isIdOnly heuristic (id present + host empty + username empty) correctly distinguishes ID-only calls from full-config calls with no false-positive risk under normal usage.
• The DB query, mapRowToConfig, and credential hydration logic are consistent with the rest of the file.
• The outer guard if (isIdOnly && config.id) is logically redundant since config.id truthiness is already part of the isIdOnly expression; the extra check only provides TypeScript type narrowing.
• The renderer payload still hardcodes authType: 'password' as a dummy field — correctly ignored server-side, but potentially misleading to future readers.
• No automated tests were added for the new code path.

Confidence Score: 4/5

• Safe to merge — the fix is logically correct and follows established patterns in the file, with only minor style issues.
• The new code path is a straightforward adaptation of the existing CONNECT handler's resolve-by-ID logic. The ID-only detection heuristic is sound for the known call sites. Two minor style issues exist (redundant guard, misleading dummy authType) but neither causes incorrect behaviour. The absence of automated tests for the new branch is the main reason the score is not a 5.
• No files require special attention; both changed files are low-risk.

Important Files Changed

Filename	Overview
src/main/ipc/sshIpc.ts	Adds ID-only resolution in the TEST_CONNECTION handler by detecting an empty host/username, querying the DB, and hydrating credentials from keytar before running the test. Logic is sound and mirrors the existing CONNECT pattern; minor: the outer if (isIdOnly && config.id) guard redundantly re-checks config.id which is already part of the isIdOnly expression.
src/renderer/components/ssh/SshConnectionTestButton.tsx	Comment-only change removing the TODO and documenting that the main process now resolves the connection by ID. The IPC payload retains a hardcoded authType: 'password' dummy field that is harmless but slightly misleading.

Sequence Diagram

sequenceDiagram
    participant R as SshConnectionTestButton
    participant IPC as TEST_CONNECTION Handler
    participant DB as SQLite via Drizzle
    participant KT as keytar CredentialService
    participant SSH as ssh2 Client

    R->>IPC: sshTestConnection with id-only payload
    IPC->>IPC: Detect isIdOnly (id set, host and username empty)
    alt isIdOnly true (new path)
        IPC->>DB: Query sshConnections by id
        DB-->>IPC: Return saved row
        IPC->>IPC: mapRowToConfig(row) into resolvedConfig
        alt authType is password
            IPC->>KT: getPassword(id)
            KT-->>IPC: stored credential
        else authType is key
            IPC->>KT: getPassphrase(id)
            KT-->>IPC: stored credential
        end
    else isIdOnly false (existing path)
        IPC->>IPC: resolvedConfig stays as original config
    end
    IPC->>SSH: connect(resolvedConfig)
    SSH-->>IPC: ready or error event
    IPC-->>R: ConnectionTestResult

Loading

Comments Outside Diff (1)

1. src/renderer/components/ssh/SshConnectionTestButton.tsx, line 39-46 (link)

   Hardcoded authType: 'password' is misleading

   The authType: 'password' value sent in the ID-only payload is a dummy placeholder that has no effect when the main process correctly resolves the config from the DB. However, any reader unfamiliar with the server-side logic may infer that this component only tests password-based connections, which is incorrect — it supports all auth types via the DB lookup.

   Consider using a sentinel string (e.g. authType: 'unknown' as any) or adding an inline comment explaining the field is ignored, to make the intent clearer:

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

_{Last reviewed commit: 14c73ca}

[greptile-apps]

Redundant config.id guard in the if condition

isIdOnly already short-circuits to a falsy value whenever config.id is falsy (it's the first operand in the && chain). The outer if (isIdOnly && config.id) therefore re-checks the same thing twice and can never be true unless config.id is already truthy.

The extra guard does help TypeScript narrow config.id to string inside the block, but that could be achieved more explicitly with a type assertion or by restructuring the guard. As written it looks like a logical mistake rather than an intentional narrowing hint.

Suggested change

	const isIdOnly =
	config.id &&
	(typeof config.host !== 'string' || !config.host.trim()) &&
	(typeof config.username !== 'string' || !config.username.trim());
	let resolvedConfig: SshConfig & { password?: string; passphrase?: string } = config;
	if (isIdOnly && config.id) {
	if (isIdOnly) {
	const id = config.id!;
	const { db } = await getDrizzleClient();
	const rows = await db
	.select({
	id: sshConnectionsTable.id,
	name: sshConnectionsTable.name,
	host: sshConnectionsTable.host,
	port: sshConnectionsTable.port,
	username: sshConnectionsTable.username,
	authType: sshConnectionsTable.authType,
	privateKeyPath: sshConnectionsTable.privateKeyPath,
	useAgent: sshConnectionsTable.useAgent,
	})
	.from(sshConnectionsTable)
	.where(eq(sshConnectionsTable.id, id))
	.limit(1);