f69c557d

build: ci keeps the tree gofmt-clean

a73x 2026-08-06 09:12

Commit message

build: ci keeps the tree gofmt-clean

gofmt is not among golangci-lint's enabled linters and nothing else checked it,
so formatting was enforced only by whoever happened to run it. Six files had
drifted — misaligned struct fields, an import out of order, spaces around a
string concatenation — while ci stayed green.

make fmt-check lists what gofmt would change and fails when that list is
non-empty. It reformats rather than diffing against HEAD, unlike the shape, api
and tidy gates: those compare a generated artifact to its committed copy, while
formatting is a property of the whole tree, and a contributor with an
unformatted file has nothing committed to compare against.

One of the six needed a hand rather than the tool. Since Go 1.19 gofmt also
reformats DOC comments, where `''` and the double-backquote are the old ASCII
spellings of typographic quotes and are rewritten as such. syncsvc/tracker.go
documents the UPDATE it mirrors, and running gofmt over it turned

	assigned_ip = CASE WHEN ?='' THEN assigned_ip ELSE ? END

into SQL with a curly quote in it — which is how the comment came to carry one
before. The fragment now sits in an indented code block, which go/doc treats as
preformatted and leaves alone. The Makefile says so where the next person will
be standing when they hit it.

Makefile

Old		New
@@ -11,7 +11,7 @@ DEADCODE_VERSION := v0.48.0
11	LINT_WARN := errcheck,revive,gocyclo,funlen,gocritic,misspell,unconvert,nakedret	11	LINT_WARN := errcheck,revive,gocyclo,funlen,gocritic,misspell,unconvert,nakedret
12		12
13	.PHONY: build build-go web test vet proto clean \	13	.PHONY: build build-go web test vet proto clean \
14	lint lint-extra arch cover tidy-check proto-check shape shape-check api api-check \	14	lint lint-extra arch cover fmt fmt-check tidy-check proto-check shape shape-check api api-check \
15	site site-check ci deadcode \	15	site site-check ci deadcode \
16	deploy release hooks site-image server-image	16	deploy release hooks site-image server-image
17		17
@@ -91,6 +91,29 @@ cover:
91	./scripts/coverage.sh	91	./scripts/coverage.sh
92		92
93	# Dependency hygiene: `go mod tidy` must produce no diff.	93	# Dependency hygiene: `go mod tidy` must produce no diff.
		94	# gofmt is not one of golangci-lint's enabled linters, so nothing was enforcing
		95	# it: six files had drifted out of canonical form while ci stayed green.
		96	#
		97	# Reformat rather than diff-against-HEAD (the shape/api/tidy pattern), because
		98	# gofmt is a whole-tree property, not a generated artifact — a contributor with
		99	# an unformatted file has nothing committed to compare against. -l lists what
		100	# would change and prints nothing when the tree is clean.
		101	#
		102	# NOTE for whoever fixes a failure here: since Go 1.19 gofmt also reformats DOC
		103	# comments, where `''` and `` are the old ASCII spellings of typographic quotes
		104	# and get rewritten as such. That is fine in prose and wrong in a SQL or shell
		105	# fragment, which stops being valid. Put such fragments in an indented code
		106	# block, which go/doc treats as preformatted and leaves alone. `gofmt -w` is
		107	# safe to run, but read what it did to your comments.
		108	fmt:
		109	gofmt -w ./cmd ./internal
		110
		111	fmt-check:
		112	@out="$$(gofmt -l ./cmd ./internal)"; \
		113	if [ -n "$$out" ]; then \
		114	echo "fmt-check: not gofmt-clean — run 'make fmt':"; echo "$$out"; exit 1; \
		115	fi
		116
94	tidy-check:	117	tidy-check:
95	go mod tidy	118	go mod tidy
96	git diff --exit-code go.mod go.sum	119	git diff --exit-code go.mod go.sum
@@ -179,7 +202,7 @@ deadcode:
179	# The merge gate. Mirrors the required checks in CI. `test` is the authoritative	202	# The merge gate. Mirrors the required checks in CI. `test` is the authoritative
180	# race-detector run; `cover` re-runs without -race to enforce the ratchet; `arch`	203	# race-detector run; `cover` re-runs without -race to enforce the ratchet; `arch`
181	# re-runs the fitness tests with -count=1 (the race run may serve them cached).	204	# re-runs the fitness tests with -count=1 (the race run may serve them cached).
182	ci: vet build-go arch lint test cover tidy-check proto-check api-check shape-check deadcode site-check	205	ci: vet build-go arch lint fmt-check test cover tidy-check proto-check api-check shape-check deadcode site-check
183		206
184	# Compile every Go package (no Node/web build needed — the embed dir ships a	207	# Compile every Go package (no Node/web build needed — the embed dir ships a
185	# placeholder, so the server builds and serves a "UI not built" notice).	208	# placeholder, so the server builds and serves a "UI not built" notice).

internal/oidcprovider/provider.go

Old		New
@@ -6,10 +6,10 @@ import (
6	"crypto/sha256"	6	"crypto/sha256"
7	"crypto/subtle"	7	"crypto/subtle"
8	"crypto/x509"	8	"crypto/x509"
		9	_ "embed"
9	"encoding/base64"	10	"encoding/base64"
10	"encoding/json"	11	"encoding/json"
11	"encoding/pem"	12	"encoding/pem"
12	_ "embed"
13	"errors"	13	"errors"
14	"html/template"	14	"html/template"
15	"net/http"	15	"net/http"

internal/server/api/auth.go

Old		New
@@ -22,10 +22,10 @@ import (
22	// addresses are passed as plain Config fields. main.go copies the config.OIDC	22	// addresses are passed as plain Config fields. main.go copies the config.OIDC
23	// block into this shape.	23	// block into this shape.
24	type OIDCConfig struct {	24	type OIDCConfig struct {
25	Issuer string	25	Issuer string
26	ClientID string	26	ClientID string
27	ClientSecret string // external confidential clients only; empty for a PKCE public client	27	ClientSecret string // external confidential clients only; empty for a PKCE public client
28	PublicURL string	28	PublicURL string
29	AllowedDomains []string // optional signup gate (case-insensitive suffix after '@')	29	AllowedDomains []string // optional signup gate (case-insensitive suffix after '@')
30	AllowedIdentities []string // optional signup gate (case-insensitive full email)	30	AllowedIdentities []string // optional signup gate (case-insensitive full email)
31	}	31	}

internal/server/api/auth_test.go

Old		New
@@ -360,9 +360,9 @@ func TestAuthCallbackMissingStateCookie(t *testing.T) {
360	// - requireSecret != "" makes /token demand that client_secret (a confidential	360	// - requireSecret != "" makes /token demand that client_secret (a confidential
361	// client), proving the secret actually flows on the external-IdP path.	361	// client), proving the secret actually flows on the external-IdP path.
362	type stubIssuer struct {	362	type stubIssuer struct {
363	url string	363	url string
364	key *rsa.PrivateKey	364	key *rsa.PrivateKey
365	kid string	365	kid string
366	email string // included as the email claim when non-empty	366	email string // included as the email claim when non-empty
367	requireSecret string // when non-empty, /token demands this client_secret	367	requireSecret string // when non-empty, /token demands this client_secret
368	unverifiedEmail bool // emit email_verified:false instead of true	368	unverifiedEmail bool // emit email_verified:false instead of true

internal/server/hosttoken/hosttoken_generation_test.go

Old		New
@@ -47,7 +47,7 @@ func TestVerifyRejectsTampering(t *testing.T) {
47	func TestVerifyRejectsMalformed(t *testing.T) {	47	func TestVerifyRejectsMalformed(t *testing.T) {
48	secret := []byte("s3cret")	48	secret := []byte("s3cret")
49	for _, cred := range []string{	49	for _, cred := range []string{
50	"", "abc123", "a.b.c", // too few parts	50	"", "abc123", "a.b.c", // too few parts
51	"abc123.deadbeef", // two parts	51	"abc123.deadbeef", // two parts
52	"a.b.c.d.e", // too many parts	52	"a.b.c.d.e", // too many parts
53	"abc123.x.170.deadbeef", // non-numeric generation	53	"abc123.x.170.deadbeef", // non-numeric generation

internal/server/store/identity.go

Old		New
@@ -61,7 +61,7 @@ func (s *Store) TenantByIdentity(issuer, subject string) (Tenant, bool, error) {
61	// the caller's tenant from a PAT or session.	61	// the caller's tenant from a PAT or session.
62	func (s *Store) TenantByID(id string) (Tenant, bool, error) {	62	func (s *Store) TenantByID(id string) (Tenant, bool, error) {
63	tn, err := scanTenant(s.db.QueryRow(	63	tn, err := scanTenant(s.db.QueryRow(
64	`SELECT ` + tenantColumns + ` FROM tenants WHERE id=?`, id))	64	`SELECT `+tenantColumns+` FROM tenants WHERE id=?`, id))
65	if errors.Is(err, sql.ErrNoRows) {	65	if errors.Is(err, sql.ErrNoRows) {
66	return Tenant{}, false, nil	66	return Tenant{}, false, nil
67	}	67	}

internal/server/syncsvc/tracker.go

Old		New
@@ -37,9 +37,11 @@ func newStatusTracker() *statusTracker {
37	// to RecordVMStatus (whose UPDATE keeps the prior ip on empty).	37	// to RecordVMStatus (whose UPDATE keeps the prior ip on empty).
38	//	38	//
39	// The tracker never decides what an address means. It caches what the write says	39	// The tracker never decides what an address means. It caches what the write says
40	// it stored, folded with that UPDATE's own empty-keeps-prior rule	40	// it stored, folded with that UPDATE's own empty-keeps-prior rule:
41	// (`assigned_ip = CASE WHEN ?='' THEN assigned_ip ELSE ? END`), so the cache	41	//
42	// holds what the row holds. An address the store DROPPED therefore leaves the	42	// assigned_ip = CASE WHEN ?='' THEN assigned_ip ELSE ? END
		43	//
		44	// so the cache holds what the row holds. An address the store DROPPED leaves the
43	// cache on the prior value rather than the rejected one, and the next report	45	// cache on the prior value rather than the rejected one, and the next report
44	// carrying a good address reads as a change and lands. Deciding the effective ip	46	// carrying a good address reads as a change and lands. Deciding the effective ip
45	// here instead — from the value the agent REPORTED rather than the one the store	47	// here instead — from the value the agent REPORTED rather than the one the store