Skip to content

ssh-mount: follow-up improvements from PR #434 review#630

Open
ambient-code[bot] wants to merge 1 commit intofeature/322-filesystem-mountfrom
fix/597-ssh-mount-followup
Open

ssh-mount: follow-up improvements from PR #434 review#630
ambient-code[bot] wants to merge 1 commit intofeature/322-filesystem-mountfrom
fix/597-ssh-mount-followup

Conversation

@ambient-code
Copy link
Copy Markdown
Contributor

@ambient-code ambient-code Bot commented Apr 23, 2026

Summary

  • Extract shared SSH utility logic (_ssh_utils.py) for temp identity file creation/cleanup, eliminating duplication between SSHWrapperClient and SSHMountClient
  • Fix LSP deviation: SSHMountClient now inherits from DriverClient instead of CompositeClient (mount is a single command, not a group)
  • Replace single 1-second wait + os.path.ismount() check with a polling loop (0.5s intervals, 10s timeout) for robustness on slow connections
  • Document allow_other mount option security implications in README
  • Restructure tests: separate _build_sshfs_args unit tests, pytest fixtures to flatten nesting, new fd-cleanup coverage, consistent side_effect for _find_executable mocks

Test plan

  • All 24 jumpstarter-driver-ssh tests pass
  • All 35 jumpstarter-driver-ssh-mount tests pass (up from 26 — added 8 _build_sshfs_args unit tests + 1 fd-cleanup test)
  • Linting passes (make lint-fix)
  • CI pipeline passes

Ref: #597

🤖 Generated with Claude Code

- Extract shared SSH utility logic into _ssh_utils.py (create_temp_identity_file,
  cleanup_identity_file) used by both SSHWrapperClient and SSHMountClient
- Fix LSP deviation: SSHMountClient now inherits from DriverClient instead of
  CompositeClient, with explicit ssh property for child access
- Replace single 1s wait + ismount check with polling loop (0.5s intervals,
  10s timeout) for mount readiness on slow connections
- Document allow_other security implications in README
- Restructure tests: separate _build_sshfs_args unit tests, use pytest fixtures
  to flatten nested patch blocks, add fd-cleanup coverage for temp identity file
  creation, use side_effect for _find_executable mocks consistently

Ref: #597

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

0 participants