refactor(server): narrow the SSH session seam and type its connection state

[otavio]

What

Narrows what the channels package needs from an SSH session down to one interface declared at the
consumer, and moves per-connection state behind typed context keys and named predicates. Both exist
to make the SSH path testable: DefaultSessionHandler, DefaultDirectTCPIPHandler and the whole
server/ssh/server/auth package had no tests before this.

Why

Session exposed 17 fields and 14 methods, and the channel handlers used 19 distinct paths into it.
Two of those reached through sess.Agent.Client — two levels into another module's struct — and the
data-copy loop read sess.Device.Info.Version to choose between Close and CloseWrite, putting a
device-compatibility rule inside a byte pump. Reaching either handler required a real SSH handshake
plus a live agent, so neither could be tested.

Per-connection state hung off gliderssh.Context under the string keys "snap" and "conn", with
progress tracked as an untyped int compared using < in four files. Both auth handlers separately
reimplemented "read the connection out of the context and close it", and password.go did not reuse
publickey.go's helper — which is why that package had no test seam at all.

Comes from the C1 and C2 candidates of an architecture review of the SSH path.

Changes

• channels: Session interface declared in the consuming package, listing the 14 operations the
  handlers actually use. Each handler splits into a thin adapter that obtains the session and a core
  that takes the interface, following the newBannerHandlerWithDeps pattern already in server.go.
  A fake is the second adapter, which is what makes the seam real rather than hypothetical.
• session: agent, client and the seat collection are unexported, so the reach-through is now
  a compile error rather than a convention. OpenAgentForwards, DialAgent and CloseAgentWrite
  replace the paths that went through them; CloseAgentWrite owns the agent-version rule.
• session: State becomes a real type with Evaluated(), Established() and String(). Typed
  context keys replace "snap" and "conn". advance() replaces four inline
  getSnapshot(ctx).save(...) calls, and stays unexported so only the step doing the work can
  declare it done.
• auth: both handlers share AuthenticableSessionOrDrop. The name states the side effect,
  because the function closes the socket on the false branch.
• tcpip: seven copies of the same log block collapse into one field set.
• tests: 16 new tests across four files, covering both channel handlers' rejection paths, the
  auth drop-guard, the agent-close policy and the connection-state module. pipe_test.go now drives
  the fake, which let its edition and namespace setup go away.

Testing

The interesting risk here is log output, not behaviour. Four changes a reviewer should agree with:

• direct-tcpip logged origin_port and origin_addr transposed, in all seven copies. Merging
  them corrected it. This was a live bug, not a refactor.
• The channel path now logs session where the session handler logged uid. Both handler guards
  already logged "session": ctx.SessionID(), so one session now reads under one key. The tree is
  split 23/24 on this pre-existing inconsistency; the rest is untouched.
• direct-tcpip logs sshid as user@namespace.device (SSHID) rather than namespace.device
  (Target.Data).
• Forward dials now abort on SSH-context cancellation, since Dial became DialContext per the Go
  conventions. A cancelled dial's error text reaches the client in the channel rejection.

Also worth a look: CloseAgentWrite on a seat with no agent channel returns ErrSeatNotFound
instead of closing. The only caller discards the error, so there is no behavioural difference today,
but it is a deliberate change from the old silent CloseWrite.

Full server suite passes (28 packages), golangci-lint run ./... reports no issues, and go mod tidy leaves the tree clean. cloud/ does not import either package, so nothing there needs to move.

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 6m 49s —— View job

---

Code Review Complete

Reviewed 21 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 medium).

The refactor is clean and self-contained. Verified along the way: the typed State predicates (Evaluated()/Established()) preserve the old comparison directions (no auth-bypass risk), the typed context keys are a hardening over the old string keys, AuthenticableSessionOrDrop closes the socket correctly on the drop path, the DialContext cancellation and CloseAgentWrite nil-guards are improvements, and the PR's "cloud/ does not import either package" claim holds (cloud/ only imports the untouched server/ssh/http and server/ssh/pkg/dialer). The direct-tcpip log-field transposition fix and the channels.Session interface conformance also check out.

The one finding is a test-effectiveness bug: the fakeSession.Recorded fake ignores the injected recordedErr, so TestPipeReportsExpectedRecordingSkips runs both table rows down an identical path and never actually exercises the wrapped ErrRecordingDisabled/ErrRecordingNoPty errors it names. See the inline comment for the one-line fix.

---

To request another review round, comment /review.
· refactor/ssh-session-seam

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 5m 59s —— View job

---

Code Review Complete

Reviewed 21 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

Verified this round: the prior finding is resolved — fakeSession.Recorded now returns the injected recordedErr, so TestPipeReportsExpectedRecordingSkips' two table rows genuinely exercise the wrapped ErrRecordingDisabled/ErrRecordingNoPty paths (dropping the %w now fails the test). The typed State predicates (Evaluated()/Established()) preserve the old comparison directions with no auth-bypass, the string→typed context-key migration ("snap"/"conn" → snapshotCtxKey{}/connCtxKey{}) is a hardening, AuthenticableSessionOrDrop closes the socket on the drop path (with an added nil guard), DialContext cancellation and the CloseAgentWrite/closesAgentOnExec nil guards are improvements, the channels.Session interface matches *session.Session method-for-method, and the direct-tcpip origin_port/origin_addr transposition fix checks out. The "cloud/ does not import either package" claim holds — cloud/ only pulls the untouched server/ssh/http and server/ssh/pkg/dialer.

---

To request another review round, comment /review.
· refactor/ssh-session-seam