docs(pkg): document every exported symbol, and delete the rest

Completes the backfill the comment rule asks for in this module: 229
exported symbols now carry a godoc, which turns the revive rule from red
to green for pkg/. Each says what a caller cannot see from the signature —
that DeviceStatusRemoved frees its slot against the device limit
immediately, that HasMaxDevicesReached compares against -1 when there is
no ceiling, that KindInvalid is what makes a forgotten scope a rejection
rather than an unbounded query.

74 other comment lines go, because no linter asks for them. What they said,
recorded here because the code no longer can:

- pkg/api/client's commonAPI and publicAPI are unexported, so the
  per-method documentation of the agent's own API went with them:
  CreateDeviceLoginCode returns a short-lived code that deep-links the
  device into the console's accept page and is authenticated with the
  device token; CreateDevicePairing is unauthenticated and the code it
  returns is itself the secret; NewReverseListenerV1 speaks RevDial and V2
  speaks Yamux.
- Retry-After is allowed to carry either a delay in seconds or a date, and
  only the delay is handled (RFC 9110 section 10.2.3).
- authorizer.code returns 0 for a role it does not know, and the switch's
  default clause leaves the permission slice empty on purpose so a role
  added later is powerless rather than privileged.
- yamux refuses to build a session without LogOutput, which is why the
  translation fills it in.
- The client's reverser field is what the SSH server dials back in through.

golangci-lint reports 0 issues for ./pkg/... with the exported rule on, and
the tests pass apart from the two asynq cases that need a Docker daemon
inside the container.

Author: otavio

pkg/api/authorizer/permissions.go

@@ -1,7 +1,12 @@
 package authorizer
 
+// Permission is one action a role may or may not perform. The values are iota-assigned and carry
+// no meaning outside the process: they are never persisted or sent on the wire, so the list can be
+// reordered.
 type Permission int
 
+// The actions a role can be granted. Grouped by resource, and deliberately finer-grained than the
+// routes: one route may require several, and a role is the set it holds.
 const (
 	DeviceAccept Permission = iota
 	DeviceReject
@@ -73,8 +78,6 @@ const (
 	SSHIdentityManage
 )
 
-// servicePermissions is intentionally empty: a service account has no management
-// permissions (see [RoleService]).
 var servicePermissions = []Permission{}
 
 var observerPermissions = []Permission{

pkg/api/authorizer/role.go

@@ -68,8 +68,6 @@ func (r Role) String() string {
 	}
 }
 
-// code converts the given role to its corresponding integer.
-// If the role is not a valid one, it returns 0.
 func (r Role) code() int {
 	switch r {
 	case RoleOwner:
@@ -101,7 +99,6 @@ func (r Role) Permissions() []Permission {
 	case RoleService:
 		permissions = servicePermissions
 	default:
-		// RoleInvalid and anything added later keep the empty slice, as the doc says.
 	}
 
 	return permissions

pkg/api/client/client.go

@@ -27,45 +27,42 @@ type publicAPI interface {
 	Endpoints() (*models.Endpoints, error)
 	AuthDevice(req *models.DeviceAuthRequest) (*models.DeviceAuthResponse, error)
 	AuthPublicKey(req *models.PublicKeyAuthRequest, token string) (*models.PublicKeyAuthResponse, error)
-	// CreateDeviceLoginCode requests a short-lived code that deep-links this device into the
-	// console's accept page. It is authenticated with the device's token.
 	CreateDeviceLoginCode(token string) (*models.DeviceLoginCode, error)
-	// GetDeviceAuthStatus reports the device's current status on the server. It is
-	// authenticated with the device's token.
 	GetDeviceAuthStatus(token string) (*models.DeviceAuthStatus, error)
-	// CreateDevicePairing submits a tenant-less agent's identity and returns a
-	// short-lived pairing code. Unauthenticated; the code is the secret.
 	CreateDevicePairing(req *models.DevicePairingRequest) (*models.DevicePairing, error)
-	// GetDevicePairingStatus polls the outcome of a pairing code. Unauthenticated.
 	GetDevicePairingStatus(code string) (*models.DevicePairingStatus, error)
-	// NewReverseListener creates a new reverse listener to be used by the Agent to connect to ShellHub's SSH server
-	// using RevDial protocol.
 	NewReverseListenerV1(ctx context.Context, token string, path string) (net.Listener, error)
-	// NewReverseListenerV2 creates a new reverse listener to be used by the Agent to connect to ShellHub's SSH server
-	// using Yamux protocol.
 	NewReverseListenerV2(ctx context.Context, token string, path string, cfg *ReverseListenerV2Config) (net.Listener, error)
 }
 
+// Client is the agent's view of the ShellHub API: the routes an agent calls, plus the reverse
+// listener it serves SSH on. Build one with NewClient.
 type Client interface {
 	commonAPI
 	publicAPI
 }
 
 type client struct {
-	scheme string
-	host   string
-	port   int
-	http   *resty.Client
-	logger *log.Logger
-	// reverser is used to create a reverse listener to Agent from ShellHub's SSH server.
+	scheme   string
+	host     string
+	port     int
+	http     *resty.Client
+	logger   *log.Logger
 	reverser reverser.Reverser
 }
 
+// ErrParseAddress is returned by NewClient when the server address is not a URL carrying scheme,
+// host and port.
 var ErrParseAddress = errors.New("could not parse the address to the required format")
 
 // NewClient creates a new ShellHub HTTP client.
 //
 // Server address must contain the scheme, the host and the port. For instance: `https://cloud.shellhub.io:443/`.
+//
+// The client retries indefinitely, backing off on the server's Retry-After when it sends one and on
+// a random delay when it does not, so an agent left running against a server that is down reconnects
+// on its own. Body-less requests go out with Content-Length: 0 rather than an empty chunked body,
+// which proxies and request binders handle far more predictably.
 func NewClient(address string, opts ...Opt) (Client, error) {
 	uri, err := url.ParseRequestURI(address)
 	if err != nil {
@@ -74,7 +71,6 @@ func NewClient(address string, opts ...Opt) (Client, error) {
 
 	const RetryAfterHeader string = "Retry-After"
 
-	// MaxRetryWaitTime is the default value for wait time between retries.
 	const MaxRetryWaitTime time.Duration = 1 * time.Hour
 
 	randomWaitTimeSecs := func() time.Duration {
@@ -95,8 +91,6 @@ func NewClient(address string, opts ...Opt) (Client, error) {
 	client.http.SetRetryCount(math.MaxInt32)
 	client.http.SetRedirectPolicy(SameDomainRedirectPolicy())
 	client.http.SetBaseURL(uri.String())
-	// Keeps body-less requests on Content-Length: 0 instead of an empty chunked body, which
-	// proxies and request binders handle far more predictably.
 	client.http.SetContentLength(true)
 	client.http.AddRetryCondition(func(r *resty.Response, err error) bool {
 		var netErr net.Error
@@ -137,9 +131,6 @@ func NewClient(address string, opts ...Opt) (Client, error) {
 				return randomWaitTimeSecs(), nil
 			}
 
-			// NOTE: The `Retry-After` supports delay in seconds and and a date time, but currently we will support only
-			// one of them.
-			// https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Retry-After
 			retryAfterSeconds, err := strconv.Atoi(retryAfterHeader)
 			if err != nil {
 				return randomWaitTimeSecs(), err

pkg/api/client/client_public.go

@@ -194,6 +194,9 @@ func (c *client) NewReverseListenerV1(ctx context.Context, token string, path st
 	return c.reverser.NewListener()
 }
 
+// ReverseListenerV2Config tunes the yamux session a v2 reverse listener multiplexes over. It is
+// the server that chooses these values and sends them to the agent, so a fleet can be retuned
+// without redeploying agents.
 type ReverseListenerV2Config struct {
 	// AcceptBacklog is used to limit how many streams may be
 	// waiting an accept.
@@ -231,6 +234,8 @@ type ReverseListenerV2Config struct {
 	StreamCloseTimeout time.Duration `json:"yamux_stream_close_timeout"`
 }
 
+// DefaultReverseListenerV2Config is what an agent uses when the server sends no configuration of
+// its own — an older server, or one that left the field empty.
 var DefaultReverseListenerV2Config = ReverseListenerV2Config{
 	AcceptBacklog:          256,
 	EnableKeepAlive:        true,
@@ -277,6 +282,9 @@ func NewReverseV2ConfigFromMap(m map[string]any) *ReverseListenerV2Config {
 	return &cfg
 }
 
+// YamuxConfigFromReverseListenerV2 translates the wire configuration into yamux's own. A nil cfg
+// yields the defaults rather than a zero-valued session, which would refuse every stream. LogOutput
+// is filled in here because yamux refuses to build a session without one.
 func YamuxConfigFromReverseListenerV2(cfg *ReverseListenerV2Config) *yamux.Config {
 	if cfg == nil {
 		cfg = &DefaultReverseListenerV2Config
@@ -290,11 +298,13 @@ func YamuxConfigFromReverseListenerV2(cfg *ReverseListenerV2Config) *yamux.Confi
 		MaxStreamWindowSize:    cfg.MaxStreamWindowSize,
 		StreamCloseTimeout:     cfg.StreamCloseTimeout,
 		StreamOpenTimeout:      cfg.StreamOpenTimeout,
-		// NOTE: LogOutput is required, and without it yamux will failed to create the session.
-		LogOutput: os.Stderr,
+		LogOutput:              os.Stderr,
 	}
 }
 
+// NewReverseListenerV2 opens the websocket to the SSH server and multiplexes it with yamux. When the
+// server's configuration is refused it retries once with DefaultReverseListenerV2Config, so an agent
+// still connects to a server whose settings it cannot satisfy.
 func (c *client) NewReverseListenerV2(ctx context.Context, token string, path string, cfg *ReverseListenerV2Config) (net.Listener, error) {
 	if token == "" {
 		return nil, errors.New("token is empty")
@@ -328,8 +338,6 @@ func (c *client) NewReverseListenerV2(ctx context.Context, token string, path st
 			"stream_open_timeout":      cfg.StreamOpenTimeout,
 		}).Error("failed to create muxed session")
 
-		// NOTE: If we fail to create the session, we should try again with the [DefaultConfig] as the client
-		// could be using different settings.
 		log.WithError(err).Warning("trying to create muxed session with default config")
 		listener, err = yamux.Server(conn, YamuxConfigFromReverseListenerV2(&DefaultReverseListenerV2Config))
 		if err != nil {

pkg/api/client/errors.go

@@ -6,12 +6,14 @@ import (
 	"net/http"
 )
 
+// Response is the part of an HTTP response the error mapping needs: everything here is decided by
+// the status code, so the body is not part of the contract.
 type Response interface {
 	StatusCode() int
 }
 
 var (
-	// ErrUnkown is returned when a non-mapped error occurred.
+	// ErrUnknown is returned when a non-mapped error occurred.
 	ErrUnknown = errors.New("unknown error")
 	// ErrConnectionFailed is returned when the client could not communicate with the sever.
 	ErrConnectionFailed = errors.New("connection failed")

pkg/api/client/logger.go

@@ -6,18 +6,24 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// LeveledLogger adapts a logrus logger to resty's leveled-logger interface, so the HTTP client's
+// own diagnostics land in the same log as everything else.
 type LeveledLogger struct {
 	Logger *logrus.Logger
 }
 
+// Errorf logs at error level. The variadic arguments are key/value pairs, not printf arguments,
+// despite the name the interface requires.
 func (l *LeveledLogger) Errorf(msg string, keysAndValues ...any) {
 	l.Logger.WithFields(toFields(keysAndValues)).Error(msg)
 }
 
+// Debugf logs at debug level, taking key/value pairs as Errorf does.
 func (l *LeveledLogger) Debugf(msg string, keysAndValues ...any) {
 	l.Logger.WithFields(toFields(keysAndValues)).Debug(msg)
 }
 
+// Warnf logs at warning level, taking key/value pairs as Errorf does.
 func (l *LeveledLogger) Warnf(msg string, keysAndValues ...any) {
 	l.Logger.WithFields(toFields(keysAndValues)).Warn(msg)
 }

pkg/api/client/options.go

@@ -8,8 +8,12 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// Opt configures a client during NewClient. An Opt that returns an error aborts construction, so
+// a client is never handed back half-configured.
 type Opt func(*client) error
 
+// WithURL sets scheme, host and port from one URL. A URL without a port gets the scheme's default
+// (443 for https, 80 otherwise) rather than port 0.
 func WithURL(u *url.URL) Opt {
 	return func(c *client) error {
 		c.scheme = u.Scheme
@@ -34,6 +38,8 @@ func WithURL(u *url.URL) Opt {
 	}
 }
 
+// WithScheme sets the scheme on its own, for a caller that builds the address in pieces rather
+// than from a URL.
 func WithScheme(scheme string) Opt {
 	return func(c *client) error {
 		c.scheme = scheme
@@ -42,6 +48,7 @@ func WithScheme(scheme string) Opt {
 	}
 }
 
+// WithHost sets the host on its own. It does not imply a port.
 func WithHost(host string) Opt {
 	return func(c *client) error {
 		c.host = host
@@ -50,6 +57,7 @@ func WithHost(host string) Opt {
 	}
 }
 
+// WithPort sets the port on its own, overriding whatever the scheme would default to.
 func WithPort(port int) Opt {
 	return func(c *client) error {
 		c.port = port
@@ -58,6 +66,7 @@ func WithPort(port int) Opt {
 	}
 }
 
+// WithLogger gives the client somewhere to log. Without it the client stays silent.
 func WithLogger(logger *logrus.Logger) Opt {
 	return func(c *client) error {
 		c.logger = logger
@@ -66,6 +75,8 @@ func WithLogger(logger *logrus.Logger) Opt {
 	}
 }
 
+// WithReverser supplies the reverse-tunnel dialer the agent listens on. Only an agent needs one;
+// an API-only client leaves it unset.
 func WithReverser(reverser reverser.Reverser) Opt {
 	return func(c *client) error {
 		c.reverser = reverser
@@ -74,6 +85,8 @@ func WithReverser(reverser reverser.Reverser) Opt {
 	}
 }
 
+// WithVersion puts the agent's version in the User-Agent header, which is how the server tells
+// which agent it is talking to and refuses ones too old for a route.
 func WithVersion(version string) Opt {
 	return func(c *client) error {
 		c.http.SetHeader("User-Agent", "shellhub-agent/"+version)

pkg/api/client/reverser.go

@@ -12,16 +12,16 @@ import (
 	"github.com/shellhub-io/shellhub/pkg/wsconnadapter"
 )
 
+// Reverser dials the SSH server over a websocket and serves the reverse tunnel the server reaches
+// the agent back through. It is the websocket implementation of reverser.Reverser.
 type Reverser struct {
 	conn *websocket.Conn
-	// host is the ShellHub's server address.
-	//
-	// It is used to create the websocket connection to the ShellHub's server.
 	host string
 }
 
 var _ reverser.Reverser = new(Reverser)
 
+// NewReverser returns a Reverser pointed at a server address. Nothing is dialed until Auth.
 func NewReverser(host string) *Reverser {
 	return &Reverser{
 		host: host,

pkg/api/client/reverser/reverser.go

@@ -6,6 +6,9 @@ import (
 	"github.com/shellhub-io/shellhub/pkg/revdial"
 )
 
+// Reverser is how the agent offers itself to the server: authenticate once, then hand back a
+// listener the server dials in on. It is its own package so the agent can depend on the seam
+// without pulling in the websocket client that implements it.
 type Reverser interface {
 	Auth(ctx context.Context, token string, connPath string) error
 	NewListener() (*revdial.Listener, error)

pkg/api/query/filter.go

@@ -8,11 +8,18 @@ import (
 )
 
 var (
-	ErrFilterInvalid         = errors.New("filter is invalid")
+	// ErrFilterInvalid is returned when a filter is not base64url-encoded JSON of the expected shape.
+	ErrFilterInvalid = errors.New("filter is invalid")
+	// ErrFilterPropertyInvalid is returned when a property node names a field the store does not
+	// allow filtering on.
 	ErrFilterPropertyInvalid = errors.New("filter property is not valid")
+	// ErrFilterOperatorInvalid is returned when a node names an operator the store does not support.
 	ErrFilterOperatorInvalid = errors.New("filter operator is not valid")
-	ErrFilterTooLarge        = errors.New("filter exceeds the maximum size")
-	ErrSorterFieldInvalid    = errors.New("sort field is not valid")
+	// ErrFilterTooLarge is returned when the encoded filter is longer than the cap, which bounds the
+	// work a single query can ask for.
+	ErrFilterTooLarge = errors.New("filter exceeds the maximum size")
+	// ErrSorterFieldInvalid is returned when a sort names a field the store does not allow sorting on.
+	ErrSorterFieldInvalid = errors.New("sort field is not valid")
 )
 
 // Filters represents a set of filters that can be applied to queries.
@@ -33,19 +40,18 @@ func NewFilters() *Filters {
 // Unmarshal decodes and unmarshals the raw filters, populating the Data attribute.
 // It rejects payloads larger than [MaxFilterRawBytes] before decode to keep
 // a hostile caller from allocating large buffers at JSON decode time.
+//
+// Both base64 alphabets are accepted, standard and URL-safe, with padding stripped first so either
+// can be tried with its unpadded decoder.
 func (fs *Filters) Unmarshal() error {
 	if len(fs.Raw) > MaxFilterRawBytes {
 		return ErrFilterTooLarge
 	}
 
-	// Strip any trailing '=' padding once so both standard and URL-safe encodings
-	// can be tried with their respective Raw (unpadded) decoders.
 	unpadded := strings.TrimRight(fs.Raw, "=")
 
 	raw, err := base64.RawStdEncoding.DecodeString(unpadded)
 	if err != nil {
-		// Fall back to RawURLEncoding (RFC 4648 §5) whose alphabet uses '-' and '_'
-		// instead of '+' and '/'.
 		raw, err = base64.RawURLEncoding.DecodeString(unpadded)
 		if err != nil {
 			return ErrFilterInvalid
@@ -59,11 +65,15 @@ func (fs *Filters) Unmarshal() error {
 	return nil
 }
 
+// Filter is one node of a query filter: a tagged union whose Type picks the shape of Params.
+// Unmarshal one rather than building it by hand, or Params holds a map instead of a params struct.
 type Filter struct {
 	Type   string `json:"type,omitempty"`
 	Params any    `json:"params,omitempty"`
 }
 
+// UnmarshalJSON decodes Params into the struct named by Type. An unrecognized Type leaves Params
+// nil rather than failing, so a filter the server does not know narrows nothing.
 func (f *Filter) UnmarshalJSON(data []byte) error {
 	var params json.RawMessage