tests & CI: -race in Makefile, cover parseJump/humanize, add govulncheck#7
Merged
Conversation
authMethods dialed the SSH_AUTH_SOCK unix socket but never closed it, so a long-running session leaked one descriptor per Dial (plus one more per jump host). The agent is only consulted during authentication, which ssh.Dial / NewClientConn complete synchronously before Dial returns, so thread a cleanup func out through clientConfig and defer it once the handshake is done. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
Every caller (connectCmd, listCmd) immediately re-sorts the result with ui.sortEntries by the user's chosen mode, so the dirs-first sort in List was never observed. Removing it also deletes the ASCII-only lessFold/toLower helpers, which disagreed with the Unicode-aware strings.ToLower used by the UI sort — a latent inconsistency that can no longer surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
PathSize swallowed every walk error and unconditionally returned nil, so the error return advertised a failure mode that could not happen and forced a dead err check at the call site. Return just int64, matching its local-filesystem twin localPathSize. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
Two helpers were copy-pasted across packages; both now live in a single internal/util package: - A byte-count formatter existed as ui.humanSize (int64, "4.2M") and update.humanBytes (int, "4.2 MB"). Both are replaced by util.HumanBytes, giving one consistent style app-wide. The only visible change is the `rtr update` download message, now "4.2M" to match the listing style. - A "~" home-expansion helper existed three times: sshx.expandHome, transfer.expandHome, and ui.expandHomeUI. All three are replaced by util.ExpandHome, using the safe variant that returns the path unchanged when $HOME is unresolved. No behavior change beyond the update-message format. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
Collapse the remote/local pane duplication and trim per-frame work: - sortEntries/sortLocalEntries now wrap a generic sortByMode, and filteredEntries/filteredLocalEntries wrap a generic filterByName, so the sort switch and the hidden+substring filter live in one place each. - cleanupTargets and remoteCleanupTargets share computeCleanup, parameterised by base/join/absent for the local-fs vs remote-SFTP cases. - commonNames was rebuilt up to four times per render (in displayedEntries, displayedLocalEntries, listLines, localListLines). Compute it once via frameCommon at the top of the frame and thread it through; the *With variants take the precomputed set. The public no-arg accessors remain for non-render callers. - The browser footer now ends in "q quit" like the other panes; helpBookmarks drops its own "q quit" so footer adds it consistently (no duplication). The public sortEntries/filtered*/displayed*/cleanupTargets signatures are unchanged, so the existing tests exercise the new shared code paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
CI runs `go test -race ./...`, but `make test` ran without the race detector, so a contributor running it locally wouldn't reproduce a race CI would catch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
Add table tests for pure, branchy helpers that lacked direct coverage: the byte formatter (unit boundaries and rollover), the ~ home expansion (prefix handling and the unresolved cases), and the ProxyJump spec parser (user/host/ port splitting, missing port, and IPv6 literals). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
Scan the dependency tree (which transitively pulls in the GitHub/GitLab/Gitea SDKs via go-selfupdate) for known vulnerabilities on every push and PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
rjayasin
force-pushed
the
claude/ui-dedupe
branch
from
June 23, 2026 04:16
744ed59 to
2bd4673
Compare
rjayasin
force-pushed
the
claude/tests-ci
branch
from
June 23, 2026 04:16
5857cb6 to
4b4009b
Compare
rjayasin
added a commit
that referenced
this pull request
Jul 14, 2026
…lncheck (#7) * sshx: close the ssh-agent socket after the handshake authMethods dialed the SSH_AUTH_SOCK unix socket but never closed it, so a long-running session leaked one descriptor per Dial (plus one more per jump host). The agent is only consulted during authentication, which ssh.Dial / NewClientConn complete synchronously before Dial returns, so thread a cleanup func out through clientConfig and defer it once the handshake is done. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sshx: drop dead listing sort from Session.List Every caller (connectCmd, listCmd) immediately re-sorts the result with ui.sortEntries by the user's chosen mode, so the dirs-first sort in List was never observed. Removing it also deletes the ASCII-only lessFold/toLower helpers, which disagreed with the Unicode-aware strings.ToLower used by the UI sort — a latent inconsistency that can no longer surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sshx: drop always-nil error from PathSize PathSize swallowed every walk error and unconditionally returned nil, so the error return advertised a failure mode that could not happen and forced a dead err check at the call site. Return just int64, matching its local-filesystem twin localPathSize. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * dedupe shared helpers into internal/util Two helpers were copy-pasted across packages; both now live in a single internal/util package: - A byte-count formatter existed as ui.humanSize (int64, "4.2M") and update.humanBytes (int, "4.2 MB"). Both are replaced by util.HumanBytes, giving one consistent style app-wide. The only visible change is the `rtr update` download message, now "4.2M" to match the listing style. - A "~" home-expansion helper existed three times: sshx.expandHome, transfer.expandHome, and ui.expandHomeUI. All three are replaced by util.ExpandHome, using the safe variant that returns the path unchanged when $HOME is unresolved. No behavior change beyond the update-message format. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ui: dedupe pane helpers, compute commonNames once, show q in footer Collapse the remote/local pane duplication and trim per-frame work: - sortEntries/sortLocalEntries now wrap a generic sortByMode, and filteredEntries/filteredLocalEntries wrap a generic filterByName, so the sort switch and the hidden+substring filter live in one place each. - cleanupTargets and remoteCleanupTargets share computeCleanup, parameterised by base/join/absent for the local-fs vs remote-SFTP cases. - commonNames was rebuilt up to four times per render (in displayedEntries, displayedLocalEntries, listLines, localListLines). Compute it once via frameCommon at the top of the frame and thread it through; the *With variants take the precomputed set. The public no-arg accessors remain for non-render callers. - The browser footer now ends in "q quit" like the other panes; helpBookmarks drops its own "q quit" so footer adds it consistently (no duplication). The public sortEntries/filtered*/displayed*/cleanupTargets signatures are unchanged, so the existing tests exercise the new shared code paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * make: run tests with -race to match CI CI runs `go test -race ./...`, but `make test` ran without the race detector, so a contributor running it locally wouldn't reproduce a race CI would catch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: cover util.HumanBytes/ExpandHome and sshx.parseJump Add table tests for pure, branchy helpers that lacked direct coverage: the byte formatter (unit boundaries and rollover), the ~ home expansion (prefix handling and the unresolved cases), and the ProxyJump spec parser (user/host/ port splitting, missing port, and IPv6 literals). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: add a govulncheck job Scan the dependency tree (which transitively pulls in the GitHub/GitLab/Gitea SDKs via go-selfupdate) for known vulnerabilities on every push and PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
PR 4 of 4 from the review stack. Base:
claude/ui-dedupe(PR #6) — merge #4 → #5 → #6 first; the diff here is only this PR's changes.make testnow runs-race, matching CI, so a contributor running it locally reproduces what CI checks.util.HumanBytes(unit boundaries and rollover),util.ExpandHome(prefix handling and unresolved cases), andsshx.parseJump(user/host/port splitting, missing port, IPv6 literals). (update.isReleaseVersionalready had a test, so it's left as-is.)govulncheckCI job scans the dependency tree (which transitively pulls in the GitHub/GitLab/Gitea SDKs via go-selfupdate) on every push and PR.go test -race ./...passes. I couldn't fully exercisegovulncheckin my sandbox (its vuln-DB fetch is blocked by restricted network — a 403), but the job is configured the standard way and will run against the live DB on GitHub Actions.🤖 Generated with Claude Code