Make CI green: clear lint backlog, gates, and the race-mode SLO

.golangci.yml

@@ -23,6 +23,17 @@ linters:
       # cancelled is a GitHub check-run conclusion value (their spelling, not ours).
       ignore-rules:
         - cancelled
+  exclusions:
+    rules:
+      # gqlgen owns the resolver method signatures: their argument names come
+      # from the schema field arguments, so a resolver that does not use one
+      # (ctx, after, dryRun, ...) cannot rename it without the next regenerate
+      # reverting the change. Do not flag unused parameters in generated
+      # resolver files.
+      - path: \.resolvers\.go$
+        linters:
+          - revive
+        text: unused-parameter
 
 formatters:
   enable:

api/graphql/connection.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/99designs/gqlgen/graphql"
+
 	"github.com/tamnd/githome/domain"
 	"github.com/tamnd/githome/presenter/gqlmodel"
 )

api/graphql/errors.go

@@ -104,7 +104,7 @@ func liftErrorTypes(next http.Handler) http.Handler {
 		}
 		// The cost walk filled the rate-limit holder during execution; write its
 		// headers here, the last point that holds the header map before flush.
-		applyRateLimitHeaders(w.Header(), r.Context())
+		applyRateLimitHeaders(r.Context(), w.Header())
 		w.Header().Set("Content-Length", strconv.Itoa(len(body)))
 		w.WriteHeader(rec.status)
 		_, _ = w.Write(body)

api/graphql/helpers.go

@@ -8,8 +8,8 @@ package graphql
 import (
 	"context"
 	"errors"
-	"sort"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 

api/graphql/loaders.go

@@ -42,8 +42,10 @@ type Loaders struct {
 
 // newLoaders constructs a fresh Loaders set for one request. batch provides
 // the underlying batch-store methods; urls and format render domain types into
-// the gqlmodel shapes the resolvers return.
-func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid.Format) *Loaders {
+// the gqlmodel shapes the resolvers return. wait is the batch coalescing window
+// every loader shares; production passes loaderWait, tests widen it so a slow,
+// loaded runner cannot break up a single logical wave into two fetches.
+func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid.Format, wait time.Duration) *Loaders {
 	return &Loaders{
 		Users: dataloader.New(func(ctx context.Context, pks []int64) (map[int64]*gqlmodel.User, error) {
 			users, err := batch.Users(ctx, pks)
@@ -55,7 +57,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid
 				out[pk] = urls.GQLUser(u, format)
 			}
 			return out, nil
-		}, loaderWait),
+		}, wait),
 
 		LabelsByIssue: dataloader.New(func(ctx context.Context, issuePKs []int64) (map[int64][]*gqlmodel.Label, error) {
 			lmap, err := batch.LabelsByIssues(ctx, issuePKs)
@@ -76,7 +78,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid
 				out[pk] = nodes
 			}
 			return out, nil
-		}, loaderWait),
+		}, wait),
 
 		AssigneesByIssue: dataloader.New(func(ctx context.Context, issuePKs []int64) (map[int64][]*gqlmodel.User, error) {
 			amap, err := batch.AssigneesByIssues(ctx, issuePKs)
@@ -92,7 +94,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid
 				out[pk] = users
 			}
 			return out, nil
-		}, loaderWait),
+		}, wait),
 
 		CommentsByIssue: dataloader.New(func(ctx context.Context, keys []commentsPreviewKey) (map[commentsPreviewKey][]*domain.Comment, error) {
 			// Group by limit so each distinct first: argument is one query.
@@ -111,7 +113,7 @@ func newLoaders(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid
 				}
 			}
 			return out, nil
-		}, loaderWait),
+		}, wait),
 	}
 }
 
@@ -133,7 +135,7 @@ func loadersFrom(ctx context.Context) *Loaders {
 // middleware stored on the context, falling back to the configured default.
 func loadersMiddleware(batch *domain.Batcher, urls *presenter.URLBuilder, format nodeid.Format, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		ctx := withLoaders(r.Context(), newLoaders(batch, urls, idFormat(r.Context(), format)))
+		ctx := withLoaders(r.Context(), newLoaders(batch, urls, idFormat(r.Context(), format), loaderWait))
 		next.ServeHTTP(w, r.WithContext(ctx))
 	})
 }

api/graphql/loaders_test.go

@@ -27,7 +27,7 @@ type countingBatchStore struct {
 	rollupCalls   atomic.Int64
 }
 
-func (c *countingBatchStore) UsersByPKs(ctx context.Context, pks []int64) (map[int64]*store.UserRow, error) {
+func (c *countingBatchStore) UsersByPKs(_ context.Context, pks []int64) (map[int64]*store.UserRow, error) {
 	c.userCalls.Add(1)
 	out := make(map[int64]*store.UserRow, len(pks))
 	for _, pk := range pks {
@@ -36,7 +36,7 @@ func (c *countingBatchStore) UsersByPKs(ctx context.Context, pks []int64) (map[i
 	return out, nil
 }
 
-func (c *countingBatchStore) LabelsByIssuePKs(ctx context.Context, pks []int64) (map[int64][]store.LabelRow, error) {
+func (c *countingBatchStore) LabelsByIssuePKs(_ context.Context, pks []int64) (map[int64][]store.LabelRow, error) {
 	c.labelCalls.Add(1)
 	out := make(map[int64][]store.LabelRow, len(pks))
 	for _, pk := range pks {
@@ -45,7 +45,7 @@ func (c *countingBatchStore) LabelsByIssuePKs(ctx context.Context, pks []int64)
 	return out, nil
 }
 
-func (c *countingBatchStore) AssigneesByIssuePKs(ctx context.Context, pks []int64) (map[int64][]int64, error) {
+func (c *countingBatchStore) AssigneesByIssuePKs(_ context.Context, pks []int64) (map[int64][]int64, error) {
 	c.assigneeCalls.Add(1)
 	out := make(map[int64][]int64, len(pks))
 	for _, pk := range pks {
@@ -54,16 +54,16 @@ func (c *countingBatchStore) AssigneesByIssuePKs(ctx context.Context, pks []int6
 	return out, nil
 }
 
-func (c *countingBatchStore) MilestonesByPKs(ctx context.Context, pks []int64) (map[int64]*store.MilestoneRow, error) {
+func (c *countingBatchStore) MilestonesByPKs(_ context.Context, _ []int64) (map[int64]*store.MilestoneRow, error) {
 	return map[int64]*store.MilestoneRow{}, nil
 }
 
-func (c *countingBatchStore) ReactionRollupsBySubjectPKs(ctx context.Context, subjectType string, pks []int64) (map[int64]store.ReactionRollup, error) {
+func (c *countingBatchStore) ReactionRollupsBySubjectPKs(_ context.Context, _ string, _ []int64) (map[int64]store.ReactionRollup, error) {
 	c.rollupCalls.Add(1)
 	return map[int64]store.ReactionRollup{}, nil
 }
 
-func (c *countingBatchStore) CommentsByIssuePKs(ctx context.Context, issuePKs []int64, perIssue int) (map[int64][]store.CommentRow, error) {
+func (c *countingBatchStore) CommentsByIssuePKs(_ context.Context, issuePKs []int64, perIssue int) (map[int64][]store.CommentRow, error) {
 	c.commentCalls.Add(1)
 	now := time.Now()
 	out := make(map[int64][]store.CommentRow, len(issuePKs))
@@ -103,7 +103,12 @@ func testLoaders(t *testing.T, cs *countingBatchStore) *Loaders {
 		SSHHost: "git.test.internal",
 		SSHPort: 22,
 	}
-	return newLoaders(domain.NewBatcher(cs), presenter.NewURLBuilder(urls), nodeid.FormatNew)
+	// The batch tests fan out N goroutines and assert they collapse into one
+	// fetch. A loaded -race runner can take longer than the 1ms production
+	// window to schedule all N, splitting the wave into two fetches and failing
+	// the count. A wide window removes that race without changing what is tested:
+	// the coalescing behavior is identical, only the deadline is generous.
+	return newLoaders(domain.NewBatcher(cs), presenter.NewURLBuilder(urls), nodeid.FormatNew, 250*time.Millisecond)
 }
 
 // TestLoaderCommentsPreviewBatches proves the fan-out shape an issue

api/graphql/nodecount.go

@@ -182,7 +182,7 @@ func (n nodeLimit) MutateOperationContext(_ context.Context, opCtx *gqlruntime.O
 }
 
 // nodeLimitExtension returns a gqlgen extension that rejects queries requesting
-// more than max nodes.
-func nodeLimitExtension(max int) gqlruntime.HandlerExtension {
-	return nodeLimit{max: max}
+// more than limit nodes.
+func nodeLimitExtension(limit int) gqlruntime.HandlerExtension {
+	return nodeLimit{max: limit}
 }

api/graphql/ratelimit.go

@@ -87,7 +87,7 @@ func rateLimitHeaders(next http.Handler) http.Handler {
 // (rejected before an operation context existed, or with no holder installed)
 // reports the one-point minimum against the full budget, matching a cost-free
 // call. It must be called before the response head is flushed.
-func applyRateLimitHeaders(h http.Header, ctx context.Context) {
+func applyRateLimitHeaders(ctx context.Context, h http.Header) {
 	used, nodes := 1, 0
 	if info := rateLimitInfoFrom(ctx); info != nil && info.filled {
 		used = info.used

api/graphql/server.go

@@ -71,7 +71,7 @@ func NewHandler(d Deps) http.Handler {
 	srv.Use(extension.Introspection{})
 	srv.Use(nodeLimitExtension(maxNodeLimit))
 	srv.Use(depthLimitExtension(maxQueryDepth))
-	var h http.Handler = liftErrorTypes(srv)
+	var h = liftErrorTypes(srv)
 	if d.Batch != nil {
 		h = loadersMiddleware(d.Batch, d.URLs, d.NodeFormat, h)
 	}

api/rest/app_contract_test.go

@@ -177,11 +177,11 @@ func TestAppObjectShape(t *testing.T) {
 		t.Fatalf("status %d, body %s", resp.StatusCode, body)
 	}
 	var app struct {
-		NodeID      string            `json:"node_id"`
-		Slug        string            `json:"slug"`
+		NodeID      string                  `json:"node_id"`
+		Slug        string                  `json:"slug"`
 		Owner       *struct{ Login string } `json:"owner"`
-		Permissions map[string]string `json:"permissions"`
-		Events      []string          `json:"events"`
+		Permissions map[string]string       `json:"permissions"`
+		Events      []string                `json:"events"`
 	}
 	decodeBody(t, body, &app)
 	if !strings.HasPrefix(app.NodeID, "A_") {
@@ -239,7 +239,7 @@ func TestRepoInstallationLookup(t *testing.T) {
 		t.Fatalf("status %d, body %s", resp.StatusCode, body)
 	}
 	var inst struct {
-		ID      int64 `json:"id"`
+		ID      int64                   `json:"id"`
 		Account *struct{ Login string } `json:"account"`
 	}
 	decodeBody(t, body, &inst)
@@ -269,9 +269,9 @@ func TestInstallationTokenScoping(t *testing.T) {
 		t.Fatalf("unscoped mint status %d, body %s", tokResp.StatusCode, tokBody)
 	}
 	var unscoped struct {
-		Token       string `json:"token"`
-		RepoSel     string `json:"repository_selection"`
-		Repos       []any  `json:"repositories"`
+		Token   string `json:"token"`
+		RepoSel string `json:"repository_selection"`
+		Repos   []any  `json:"repositories"`
 	}
 	decodeBody(t, tokBody, &unscoped)
 	if unscoped.RepoSel != "all" {
@@ -290,7 +290,7 @@ func TestInstallationTokenScoping(t *testing.T) {
 		t.Fatalf("scoped mint status %d, body %s", scResp.StatusCode, scBody)
 	}
 	var scoped struct {
-		RepoSel string `json:"repository_selection"`
+		RepoSel string                  `json:"repository_selection"`
 		Repos   []struct{ Name string } `json:"repositories"`
 	}
 	decodeBody(t, scBody, &scoped)
@@ -355,7 +355,7 @@ func TestInstallationReposSelected(t *testing.T) {
 		t.Fatalf("status %d, body %s", resp.StatusCode, body)
 	}
 	var env struct {
-		TotalCount int `json:"total_count"`
+		TotalCount int                     `json:"total_count"`
 		Repos      []struct{ Name string } `json:"repositories"`
 	}
 	decodeBody(t, body, &env)

api/rest/archives_contract_test.go

@@ -24,7 +24,7 @@ func getNoRedirect(t *testing.T, srv *httptest.Server, path string) (*http.Respo
 	if err != nil {
 		t.Fatalf("GET %s: %v", path, err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		t.Fatalf("read %s: %v", path, err)
@@ -73,7 +73,7 @@ func readZipEntries(t *testing.T, body []byte) map[string]string {
 			t.Fatalf("open %s: %v", f.Name, err)
 		}
 		data, err := io.ReadAll(rc)
-		rc.Close()
+		_ = rc.Close()
 		if err != nil {
 			t.Fatalf("read %s: %v", f.Name, err)
 		}
@@ -89,7 +89,7 @@ func readTarGzEntries(t *testing.T, body []byte) map[string]string {
 	if err != nil {
 		t.Fatalf("open gzip: %v", err)
 	}
-	defer gzr.Close()
+	defer func() { _ = gzr.Close() }()
 	tr := tar.NewReader(gzr)
 	out := map[string]string{}
 	for {

api/rest/contents_write.go

@@ -173,7 +173,7 @@ func currentBlobSHA(d Deps, repo *domain.Repo, path, branch string) (string, boo
 	if err != nil || res.IsDir {
 		return "", false
 	}
-	return string(res.Entry.SHA), true
+	return res.Entry.SHA, true
 }
 
 // errShaRequired is GitHub's 422 for a contents PUT or DELETE that targets an

api/rest/git_objects_write.go

@@ -136,10 +136,10 @@ func handleGitCommitCreate(d Deps) mizu.Handler {
 			return err
 		}
 		var body struct {
-			Message   string   `json:"message"`
-			Tree      string   `json:"tree"`
-			Parents   []string `json:"parents"`
-			Author    *struct {
+			Message string   `json:"message"`
+			Tree    string   `json:"tree"`
+			Parents []string `json:"parents"`
+			Author  *struct {
 				Name  string `json:"name"`
 				Email string `json:"email"`
 				Date  string `json:"date"`

api/rest/issues.go

@@ -483,27 +483,6 @@ func handleCommentEdit(d Deps) mizu.Handler {
 	}
 }
 
-// handleCommentDelete serves DELETE /repos/{owner}/{repo}/issues/comments/{id}.
-func handleCommentDelete(d Deps) mizu.Handler {
-	return func(c *mizu.Ctx) error {
-		id, ok := pathInt64(c, "id")
-		if !ok {
-			writeError(c.Writer(), errNotFound())
-			return nil
-		}
-		actor := auth.ActorFrom(c.Request().Context())
-		err := d.Issues.DeleteComment(c.Request().Context(), actor.UserID, c.Param("owner"), c.Param("repo"), id)
-		if issueError(c.Writer(), err) {
-			return nil
-		}
-		if err != nil {
-			return err
-		}
-		c.Writer().WriteHeader(http.StatusNoContent)
-		return nil
-	}
-}
-
 // handleIssueDeleteDispatch dispatches the two DELETE shapes that share
 // /issues/{seg1}/{seg2} and that mizu cannot tell apart without a dispatcher,
 // because neither "/issues/comments/{id}" nor "/issues/{number}/labels" is
@@ -1230,12 +1209,3 @@ func queryInt64(c *mizu.Ctx, name string) (int64, bool) {
 	}
 	return n, true
 }
-
-// pageNum reads the 1-based page query, defaulting to 1.
-func pageNum(c *mizu.Ctx) int {
-	n, err := strconv.Atoi(c.Query("page"))
-	if err != nil || n <= 0 {
-		return 1
-	}
-	return n
-}