test: ssh-tunnel — extractSshConfig validation and lifecycle safety#504
Open
anandgupta42 wants to merge 1 commit intomainfrom
Open
test: ssh-tunnel — extractSshConfig validation and lifecycle safety#504anandgupta42 wants to merge 1 commit intomainfrom
anandgupta42 wants to merge 1 commit intomainfrom
Conversation
Add 8 tests for the SSH tunnel module (zero prior coverage). These tests verify extractSshConfig correctly handles defaults, connection_string conflicts, and key/password passthrough, plus confirm closeTunnel and closeAllTunnels are safe no-ops when no tunnels exist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> https://claude.ai/code/session_01NiGUekwkcSnAAuncsehdFn
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughA new test suite for SSH tunnel configuration extraction and lifecycle management is introduced. Tests validate parameter extraction, default value application, error handling for conflicting configurations, and tunnel closure operations. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opencode/test/altimate/ssh-tunnel.test.ts (1)
73-77: Avoid PEM-shaped test fixtures to reduce secret-scanner false positives.Using
"BEGIN RSA PRIVATE KEY"in tests may trigger credential scanners. A neutral placeholder still validates pass-through behavior with less CI noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/altimate/ssh-tunnel.test.ts` around lines 73 - 77, The test uses a PEM-shaped fixture ("BEGIN RSA PRIVATE KEY") which can trigger secret scanners; change the fixture passed for ssh_private_key to a neutral placeholder (e.g., "SSH-PRIVATE-KEY-FAKE") and update the assertion that checks ssh_private_key (currently expect(result!.ssh_private_key).toContain("BEGIN RSA PRIVATE KEY")) to assert the new placeholder (e.g., equality or toContain "SSH-PRIVATE-KEY-FAKE"); keep the ssh_password check (expect(result!.ssh_password).toBe("also-provided")) as-is. Ensure you update the test setup where ssh_private_key is provided and the corresponding assertion that verifies the pass-through on the result object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/test/altimate/ssh-tunnel.test.ts`:
- Around line 81-91: Tests rely on shared module state in activeTunnels, so make
lifecycle tests deterministic by resetting that state before and/or after each
test: inside the describe("SSH tunnel: lifecycle helpers (no real SSH)") block
add a beforeEach(() => closeAllTunnels()) (and optionally afterEach(() =>
closeAllTunnels())) so that activeTunnels is cleared prior to each invocation of
closeTunnel/getActiveTunnel; use the existing closeAllTunnels() helper (or
directly clear activeTunnels if needed) and ensure the helper is
imported/accessible in the test file.
---
Nitpick comments:
In `@packages/opencode/test/altimate/ssh-tunnel.test.ts`:
- Around line 73-77: The test uses a PEM-shaped fixture ("BEGIN RSA PRIVATE
KEY") which can trigger secret scanners; change the fixture passed for
ssh_private_key to a neutral placeholder (e.g., "SSH-PRIVATE-KEY-FAKE") and
update the assertion that checks ssh_private_key (currently
expect(result!.ssh_private_key).toContain("BEGIN RSA PRIVATE KEY")) to assert
the new placeholder (e.g., equality or toContain "SSH-PRIVATE-KEY-FAKE"); keep
the ssh_password check (expect(result!.ssh_password).toBe("also-provided"))
as-is. Ensure you update the test setup where ssh_private_key is provided and
the corresponding assertion that verifies the pass-through on the result object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 46ba7f5c-a48f-497a-ad18-367ed5ea4e4b
📒 Files selected for processing (1)
packages/opencode/test/altimate/ssh-tunnel.test.ts
Comment on lines
+81
to
+91
| describe("SSH tunnel: lifecycle helpers (no real SSH)", () => { | ||
| test("closeTunnel is a no-op for unknown name", () => { | ||
| closeTunnel("nonexistent-tunnel-name") | ||
| expect(getActiveTunnel("nonexistent-tunnel-name")).toBeUndefined() | ||
| }) | ||
|
|
||
| test("closeAllTunnels is safe when no tunnels exist", () => { | ||
| closeAllTunnels() | ||
| expect(getActiveTunnel("any")).toBeUndefined() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Make lifecycle tests deterministic by resetting shared tunnel state.
activeTunnels is module-level shared state, so these tests can become order-dependent when run with other tests touching tunnel lifecycle. Add setup/teardown cleanup to lock test preconditions.
🔧 Proposed fix
-import { describe, test, expect } from "bun:test"
+import { describe, test, expect, beforeEach, afterEach } from "bun:test"
describe("SSH tunnel: lifecycle helpers (no real SSH)", () => {
+ beforeEach(() => {
+ closeAllTunnels()
+ })
+
+ afterEach(() => {
+ closeAllTunnels()
+ })
+
test("closeTunnel is a no-op for unknown name", () => {
closeTunnel("nonexistent-tunnel-name")
expect(getActiveTunnel("nonexistent-tunnel-name")).toBeUndefined()
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/test/altimate/ssh-tunnel.test.ts` around lines 81 - 91,
Tests rely on shared module state in activeTunnels, so make lifecycle tests
deterministic by resetting that state before and/or after each test: inside the
describe("SSH tunnel: lifecycle helpers (no real SSH)") block add a
beforeEach(() => closeAllTunnels()) (and optionally afterEach(() =>
closeAllTunnels())) so that activeTunnels is cleared prior to each invocation of
closeTunnel/getActiveTunnel; use the existing closeAllTunnels() helper (or
directly clear activeTunnels if needed) and ensure the helper is
imported/accessible in the test file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What does this PR do?
1.
extractSshConfigand tunnel lifecycle —src/altimate/native/connections/ssh-tunnel.ts(8 new tests)This module handles SSH tunnel creation, configuration extraction, and lifecycle management for remote database connections. Zero tests existed prior to this PR. The
extractSshConfigfunction is called on every connection creation inregistry.ts— if it mishandles edge cases (missing port defaults, connection_string conflicts), users get cryptic errors or misconfigured tunnels.New coverage includes:
nullwhen nossh_hostis present (skip tunnel path)ConnectionConfigssh_portdefaults to22,ssh_userdefaults to"root",hostdefaults to"127.0.0.1",portdefaults to5432ssh_hostis used withconnection_string(these are mutually exclusive — SSH tunnels need host/port to rewrite)ssh_private_keyandssh_passwordare passed through tostartTunnel(which decides priority)closeTunnelandcloseAllTunnelsare safe no-ops when no tunnels exist (no crashes on the sharedactiveTunnelsMap)Discovery process
ssh-tunnel.tsas a critical infrastructure module with zero test coverageType of change
Issue for this PR
N/A — proactive test coverage for SSH tunnel infrastructure
How did you verify your code works?
Checklist
https://claude.ai/code/session_01NiGUekwkcSnAAuncsehdFn
Summary by CodeRabbit