feat: add configurable debug logging - #23
Merged
Merged
Conversation
uhs-robert
force-pushed
the
feat/debug-logging
branch
from
August 30, 2026 12:26
9390d8f to
1c14833
Compare
There was a problem hiding this comment.
馃煛 Changes recommended
The logger currently creates log directories without restrictive permissions and can spam notifications on write failure, which should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces centralized, opt-in debug logging to help diagnose SSH/SSHFS connection and mount issues in sshfs.nvim, along with runtime toggling via a new :SSHDebug command and accompanying documentation.
Changes:
- Added a
debugconfig section (enabled,log_file) and documented it in the README and new docs page. - Implemented a new
sshfs.lib.loggermodule and integrated debug/error logging into SSH authentication, ControlMaster management, remote-home resolution, and SSHFS mount execution. - Added
:SSHDebug [on|off]to toggle debug logging for the current Neovim session.
File summaries
| File | Description |
|---|---|
| README.md | Documents new debug config and :SSHDebug command; links to debug logging docs. |
| lua/sshfs/lib/sshfs.lua | Adds mount/auth workflow logging (success/failure, stdout/stderr). |
| lua/sshfs/lib/ssh.lua | Adds logging around socket dir creation, auth flows, ControlMaster cleanup, and remote-home probing. |
| lua/sshfs/lib/logger.lua | Introduces the centralized logger implementation (enable/disable/toggle + file writes). |
| lua/sshfs/init.lua | Adds initialization-time debug log entry. |
| lua/sshfs/config.lua | Adds default debug configuration keys. |
| lua/sshfs/api.lua | Adds Api.debug() and registers :SSHDebug command with completion. |
| docs/debug-logging.md | Adds dedicated documentation for enabling and using debug logging. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Lite
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+16
to
+22
| local function ensure_parent_dir(path) | ||
| local parent = vim.fn.fnamemodify(path, ":h") | ||
| if parent ~= "" and vim.fn.isdirectory(parent) == 0 then | ||
| local result = vim.fn.mkdir(parent, "p") | ||
| if result == 0 and vim.fn.isdirectory(parent) == 0 then error("could not create log directory: " .. parent) end | ||
| end | ||
| end |
Comment on lines
+40
to
+44
| local function notify_write_failure(err) | ||
| vim.schedule(function() | ||
| vim.notify("sshfs.nvim: failed to write debug log: " .. tostring(err), vim.log.levels.ERROR) | ||
| end) | ||
| end |
|
|
||
| With no argument, `:SSHDebug` toggles logging. The command reports the active log path. | ||
|
|
||
| When enabled, sshfs.nvim records timestamped diagnostics for SSH authentication, ControlMaster setup/cleanup, remote home resolution, and SSHFS mount subprocesses, including exit codes, stdout, and stderr. No log file writes are performed while debug logging is disabled. |
uhs-robert
force-pushed
the
feat/debug-logging
branch
from
August 30, 2026 12:37
8ad8ec9 to
7b5c6f3
Compare
uhs-robert
force-pushed
the
feat/debug-logging
branch
from
August 30, 2026 12:43
1d1fbf8 to
37f0653
Compare
Logger.is_enabled and Logger.path could be reached before Config.setup had populated Config.options, which would index a nil config. Tolerate a missing configuration, parenthesize escape_log_value so the gsub match count is not returned to callers, and export App.debug alongside the other API entry points.
Adds regression coverage for the logging introduced here, on top of the shared harness. Covers the gate: nothing is written while logging is off, not even the file, and the logger tolerates being reached before setup has run. Covers runtime toggling through :SSHDebug in all four argument forms, including that an unknown argument changes nothing and that toggling never rewrites user configuration. Covers the record format: timestamp, level, message, context rendered in a stable order, multi-line subprocess output escaped onto a single line, appending across calls, log directory creation, and owner-only file permissions, since logs can carry hostnames and SSH output. Covers write failures being reported once rather than on every call and never raising out of the caller. Covers what a connection attempt actually records: the mount attempt, the exit code and stderr of a failed mount, the fallback from batch to interactive authentication with its SSH reason, and an authentication failure, plus that none of it is written when logging is off. Refs #24
# Conflicts: # lua/sshfs/init.lua # lua/sshfs/lib/ssh.lua # lua/sshfs/lib/sshfs.lua
Remote stderr was written to the log verbatim, so a hostile server could plant terminal escape sequences that fire when the file is read. The log also grew without limit, and the four command-line call sites paid for their context string even with logging off.
uhs-robert
marked this pull request as ready for review
September 11, 2026 01:43
# Conflicts: # lua/sshfs/lib/ssh.lua # lua/sshfs/lib/sshfs.lua
# Conflicts: # lua/sshfs/lib/sshfs.lua
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
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
Adds centralized, opt-in debug logging for SSH/SSHFS diagnostics.
debug.enabledanddebug.log_fileconfiguration:SSHDebug,:SSHDebug on, and:SSHDebug offdocs/debug-logging.mdDefault log path:
Motivation
Issue #9 exposed that connection failures could be difficult to diagnose because the relevant SSH/SSHFS subprocess details were not retained. This provides a low-overhead diagnostic path without making normal plugin usage noisy.
Testing
Regression coverage is included, on top of the shared harness from #25. Run it with
make test.setup():SSHDebugin all four argument forms, with an unknown argument changing nothing and toggling never rewriting user configurationStill not runtime-tested against a live SSH/SSHFS host; kept as draft for that verification.
Closes #11