Conversation
- new test.yaml: go vet + go test on PRs and master pushes, also callable as a reusable workflow. - release.yml: gate goreleaser on the test workflow via needs. - flake.nix: doCheck = true so cachix.yaml's nix build runs go test as part of the build (gates the cache push automatically). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The docs/ directory has been empty for a while. Remove the layout entry until we write proper docs (tracked separately). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace strings.Index hunting for `"tag_name":"` with json.NewDecoder into a typed struct. Same correctness, no quirks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames clipboard.go to clipboard_darwin.go (file-suffix makes it auto-darwin) and adds clipboard_other.go with stub functions returning errUnsupported on every other GOOS. The remote-shim path on Linux never calls into clipboard, so this is a no-op for current users while unblocking a clean build matrix and a future Linux client backend. Test file similarly renamed; the now-redundant runtime.GOOS skip is removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five sites repeated ssh + bash + script-via-stdin boilerplate: prepareRemote, detectRemoteDesktop, and the trio in infectRemote (mkdir, infect self, command -v xclip). Consolidate into one helper that always uses BatchMode=yes and pipes stderr to os.Stderr so SSH errors surface to the user. Behavior change worth noting: the three infectRemote sites now use BatchMode=yes — password-auth-only setups (rare) will see "Permission denied (publickey)" instead of repeated password prompts. Pubkey auth that works for the initial scp works for every subsequent ssh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three sites duplicated the "marshal envelope, choose inline vs attachment by inlineThreshold and image-mime, call ntfy.Publish*" flow: clipboard.go's clip-read success and error responses, and shim.go's clip-write request. Consolidate behind wire.Publish, which takes the envelope by value so the inline-mode mutation of Body doesn't leak back to the caller (callers immediately log the envelope after publish, and the log shouldn't include base64 noise). inlineThreshold moves into wire as the exported InlineThreshold — the inline-vs-attachment decision is a protocol concern that lives naturally with the envelope type. wire now imports ntfy; both packages remain leaves of cmd/nssh. Tests cover both transport paths, the large-text → attachment threshold, the image → attachment unconditional rule, the nil-data case (kind=open), and the no-mutation contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Until now, both the writer (logEvent + logMessage) and the reader (status.go::formatEvent / formatWireMessage) handled events as map[string]any with stringly-typed field names. Renaming a field or adding one was grep-and-pray. LogEvent in log.go is now the shared schema. omitempty drops fields that aren't relevant to a given event, so the on-disk JSON shape is unchanged. Exit and Mosh are *int / *bool so callers can record an explicit zero/false (session-end exit=0 success) without being dropped. Updated five logEvent call sites (main.go × 3, shim.go × 6 — wait, six in shim.go, three in main.go) to pass typed LogEvent struct literals. logMessage and the inline session-open emitter in prepareRemote use the same type. Reader unmarshals straight into LogEvent, with the per-event-detail fields rendered alphabetically to preserve current `nssh status --tail` output. Adds log_test.go covering: zero-fields omitted, explicit Exit=0 and Mosh=true preserved, full round-trip including pointer fields, and the wire-message subset emitted by logMessage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
main.go was 507 lines mixing CLI dispatch, version printing, OAuth proxy, ntfy subscriber loop, message routing, prepareRemote, and the session exec lifecycle. Split into: - main.go (82 lines): main, usage, printVersion, buildVersion — pure CLI dispatch. - session.go (new, 288 lines): nsshMain plus the runtime guts — subscribeNtfy, deadlineConn, handleMessage, runSession, resetTerminal, remoteHasMosh. - oauth.go (new, 83 lines): localhostRe, extractLocalhostPort, proxyOAuthCallback, handleOpen. - remote.go: now also holds prepareRemote and resolveShortHost alongside runRemoteScript. - infect.go: gains infectCmd (the subcommand parser belongs with infectSelf and infectRemote). No behavior change. Each function moved verbatim; doc comments on the moved-in functions were tightened but otherwise the implementations are identical. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four small follow-ups from self-review of the cleanup branch: - oauth: cap proxyOAuthCallback's Accept at 5 minutes via a TCPListener deadline. Previously, an abandoned OAuth flow (user closes the tab) leaked the listener + goroutine until the nssh session ended. - session: handleMessage now decodes the inline base64 body once and passes it to handleClipWrite; the handler's own decode block is gone. Cheap (≤3KB inline payload max) but the redundancy was visible. - session: extract selectTransport from nsshMain — flag-driven ssh-vs-mosh choice plus locale env-setup is now a focused helper. - clipboard: drop the local-task-id reference from clipboard_other.go's package doc; replace with a more durable forward-pointer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hand-rolled looksLikeSemver was a strict shape check (vX.Y.Z, no prerelease, no build metadata) that doubled as the gate for the session-start version-mismatch nag. The string-equality compare on mismatch was a latent bug: v1.2.3+build1 vs v1.2.3+build2 would fire even though they're the same release. Drop in golang.org/x/mod/semver, rename the gate to isReleaseVersion (more honest about its actual semantics — strictly stricter than semver.IsValid because we exclude prereleases and build metadata), and replace the != string compare with semver.Compare so build metadata is ignored. Behavior change: a release client connecting to a remote running a +dirty dev build of the same release tag no longer prompts to overwrite. Also relax CLAUDE.md's "stdlib only" rule to "minimize external deps; prefer stdlib + golang.org/x/* over third-party." Pulls in x/mod and (transitively) x/tools — both Go-team maintained. flake.nix vendorHash flipped from null to the computed hash now that go.mod has a real require. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two focused docs to capture the things that aren't obvious from the code or the README: - docs/internals.md (~260 lines): architecture, end-to-end flows for clipboard paste and OAuth callback, and the reasoning behind the unusual choices (ntfy as side channel, argv[0] dispatch, topic as secret, infect-self desktop refusal, what we deliberately don't do). - docs/protocol.md (~190 lines): wire envelope schema, inline-vs- attachment rules, the four kinds, log event vocabulary, config precedence, state directory layout, ntfy endpoints touched. CLAUDE.md regains the docs/ entry. README links both at the bottom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spell out exactly which kinds of changes need a docs/ touch (new envelope kinds, new LogEvent fields, config keys, ntfy endpoints, personas) and which don't (pure refactors). Goal: keep internals.md/protocol.md from drifting by treating doc updates as part of the change, not a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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
11 commits of focused cleanup. No user-visible behavior changes except where explicitly called out.
What changed
Refactors / dedup
main.go(507 lines) split intomain.go(CLI dispatch, 82 lines),session.go(orchestration + ntfy subscriber),oauth.go(callback proxy), and grewremote.go(SSH helpers) andinfect.go(subcommand parser).wire.Publishhelper consolidates 3 sites that duplicated the marshal-envelope-and-pick-inline-vs-attachment dance.runRemoteScripthelper consolidates 5 sites that calledssh ... bash -l -swith a script over stdin.LogEventstruct types the JSONL log schema — both writer (logEvent/logMessage/prepareRemote) and reader (status.go::formatEvent) marshal against the same type now.logTopic,logErr,shortPath.Correctness
golang.org/x/mod/semverreplaces hand-rolledlooksLikeSemverand string-equality version compare. Fixes a latent false-positive:v1.2.3+build1vsv1.2.3+build2no longer triggers the upgrade nag. Behavior change worth noting: a release client connecting to a remote running a+dirtydev build of the same release no longer prompts to overwrite.proxyOAuthCallbacknow capsAcceptat 5 minutes via a TCPListener deadline. Previously, an abandoned OAuth flow leaked the listener + goroutine until session end.latestReleaseTagparses the GitHub response withencoding/jsoninstead of hunting for\"tag_name\":\"withstrings.Index.Cross-platform
internal/clipboardgated toGOOS=darwinvia filename suffix;clipboard_other.goprovides stub functions returningerrUnsupportedso the linux remote-shim build compiles cleanly.runRemoteScriptchange: threeinfectRemotessh sites now run withBatchMode=yes. Password-auth-only setups will seePermission denied (publickey)instead of repeated prompts. Pubkey auth that worked for the initial scp works for every subsequent ssh.CI
.github/workflows/test.yamlrunsgo vet+go teston PRs and master pushes; reusable viaworkflow_call.release.ymlnowneeds: testso goreleaser is gated on green tests.flake.nixsetsdoCheck = true, so the cachix build runsgo testas part ofnix build.Tests
wire.Publish(5 cases — inline / image-attachment / large-text / no-mutate / nil-data),LogEventschema (4 cases — omit-zeros, preserve explicit zero/false, round-trip, wire subset).cmd/nsshhad zero tests; now haslog_test.go. Broader test coverage tracked separately.Test plan
go test ./...passes on darwingo vet ./...andstaticcheck ./...cleanGOOS=linux GOARCH=amd64 go buildsucceedsnix buildsucceeds (test job runs underdoCheck = true)nssh devbox, paste image,xdg-open, OAuth callback flowFiles
🤖 Generated with Claude Code
Note
Medium Risk
Medium risk due to significant refactoring of session/infect/message-handling code and changes to version comparison and remote SSH invocation (
BatchMode=yes), which could affect install/upgrade prompts and remote automation behavior.Overview
Restructures the
nsshCLI implementation by splitting the former largemain.gointo focused modules (session.go,oauth.go,remote.go) and movinginfectargument parsing intoinfect.go, while keeping the same high-level flows.Deduplicates wire/transport plumbing by introducing
wire.Publish(central inline-vs-attachment logic) andrunRemoteScript(sharedssh ... bash -l -shelper), and updates clipboard/shim code to reuse these helpers and avoid repeated base64/JSON handling.Tightens correctness around versions, OAuth, and logging: replaces hand-rolled semver checks with
golang.org/x/mod/semver(including semver-based remote/local mismatch checks), adds a 5-minute accept deadline to OAuth callback proxying, switches GitHub release tag parsing toencoding/json, and replaces map-based log events with a typedLogEventschema that is used by both log writer andstatus --tailformatter (with new unit tests).Improves build/CI signal and docs by adding a reusable
testGitHub workflow and gating releases on it, enablingdoCheckinflake.nix, adding non-darwin clipboard stubs for cross-compilation, and adding newdocs/internals.md+docs/protocol.mdreferences from README/CLAUDE.Reviewed by Cursor Bugbot for commit d129354. Bugbot is set up for automated code reviews on this repo. Configure here.