Skip to content

fix: honor ssh_key_path config in SessionKeyManager and config.local.…#139

Merged
jahwag merged 4 commits into
jahwag:masterfrom
cchervitz:fix/ssh-key-path-config
Mar 8, 2026
Merged

fix: honor ssh_key_path config in SessionKeyManager and config.local.…#139
jahwag merged 4 commits into
jahwag:masterfrom
cchervitz:fix/ssh-key-path-config

Conversation

@cchervitz

Copy link
Copy Markdown
Contributor

Problem

Setting ssh_key_path in .claudesync/config.local.json has no effect.
SessionKeyManager is instantiated without any arguments, and _find_ssh_key()
only searches ~/.ssh/ for hardcoded filenames (id_ed25519, id_ecdsa).

Users with custom-named keys (e.g., ~/.ssh/my_claude_key) are prompted to
enter a path on every run, even with a valid config entry.

Fix

  • SessionKeyManager.__init__ now accepts an optional ssh_key_path parameter
  • _find_ssh_key() checks the configured path first:
    1. Direct file path → use immediately
    2. Directory → search it alongside ~/.ssh/
    3. Default ~/.ssh/ fallback with standard key names
    4. Prompt user as last resort (with improved guidance)
  • FileConfigManager now passes self.get("ssh_key_path") when
    instantiating SessionKeyManager
  • Improved prompt message clarifies that keys should be named id_ed25519
    if not using a custom ssh_key_path config

Files Changed

  • src/claudesync/session_key_manager.py
  • src/claudesync/configmanager/file_config_manager.py

@cchervitz cchervitz requested a review from jahwag as a code owner February 15, 2026 16:58

@jahwag jahwag left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

checks are failing

@cchervitz

Copy link
Copy Markdown
Contributor Author

Sorry! My 1st PR. Didn't see that it required flake & black checks. I think everything should pass now!

@jahwag jahwag linked an issue Feb 27, 2026 that may be closed by this pull request

@jahwag jahwag left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hey, nice fix! This solves a real annoyance reported in #82. Couple things before merging:

The _find_ssh_key method has a bunch of new branches now but no tests. Would be
great to add at least a few covering the main paths (configured file, configured
dir, default fallback).

Also the new user-facing messages use print() but the class already has
self.logger.

Minor thing: the check on line 47 that tries to guess whether a non-existent
path is a file or directory (configured.suffix or
configured.name.startswith("id_")) could misfire for keys with unusual names.
Probably better to just always treat it as a missing file if it doesn't exist.

One more thing: commits need to be signed for this repo. Could you rebase with
signed commits?

Comment thread src/claudesync/session_key_manager.py Outdated
@cchervitz cchervitz force-pushed the fix/ssh-key-path-config branch from d1860fc to b48ac13 Compare February 28, 2026 04:28
@cchervitz

Copy link
Copy Markdown
Contributor Author

Thanks for the great suggestions. I signed my files, cleaned up that duplicate comment, added some tests, used self-logger, and agreed with your simplified approach on checking file vs directory. Let me know if it looks good to you!

@cchervitz cchervitz requested a review from jahwag March 3, 2026 19:42
@jahwag jahwag merged commit f949bd5 into jahwag:master Mar 8, 2026
3 checks passed
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.

Custom ssh keyfile name is not saved

2 participants