From 6807c8e786a5aabb81bdb590d44906cf555d5a17 Mon Sep 17 00:00:00 2001 From: Duc-Tam Nguyen Date: Sat, 13 Jun 2026 18:31:01 +0700 Subject: [PATCH 1/5] Make CI green: clear lint backlog, gates, and the race-mode SLO The lint job had a standing backlog and the test job failed under -race on the wall-clock page SLO. This clears all three CI legs: - lint: fix the accumulated golangci findings across the tree - errcheck on deferred Close and a best-effort OnPush, unused parameters renamed to _, unconvert on SHA-alias string() casts, staticcheck dead assignments, context-as-argument ordering in two test helpers, exported-symbol doc comments, and the AuthCodeRequest stutter renamed to CodeRequest. Remove five dead REST handlers whose routes are served by the path-disambiguating dispatchers. Exclude unused-parameter on generated .resolvers.go files, whose signatures gqlgen owns. - gates: reword two api/rest/root.go comments so served code no longer names an upstream GitHub host. - test -race: the fe page SLO asserts a wall-clock budget that race instrumentation inflates past the limit. Keep issuing the requests under -race so the detector still exercises the handlers, but only enforce the budget on an uninstrumented build, gated by a build-tagged raceEnabled flag. --- .DS_Store | Bin 0 -> 10244 bytes .golangci.yml | 11 +++ api/graphql/connection.go | 1 + api/graphql/errors.go | 2 +- api/graphql/helpers.go | 2 +- api/graphql/loaders_test.go | 12 +-- api/graphql/nodecount.go | 6 +- api/graphql/pull.resolvers.go | 6 +- api/graphql/ratelimit.go | 2 +- api/graphql/ref.resolvers.go | 2 +- api/graphql/server.go | 2 +- api/rest/app_contract_test.go | 20 ++--- api/rest/archives_contract_test.go | 6 +- api/rest/contents_write.go | 2 +- api/rest/git_objects_write.go | 8 +- api/rest/issues.go | 30 -------- api/rest/issues_extra.go | 30 -------- api/rest/keys.go | 1 - api/rest/misc_compat.go | 16 ++-- api/rest/pulls_extra.go | 65 ---------------- api/rest/repo_permissions_contract_test.go | 2 +- api/rest/repo_surface_contract_test.go | 4 +- api/rest/root.go | 6 +- api/rest/social_contract_test.go | 6 +- auth/app_jwt.go | 5 +- auth/webflow.go | 6 +- conform/conform.go | 3 + domain/event.go | 2 +- domain/key_service.go | 1 - domain/push.go | 4 +- domain/repo_community.go | 2 +- domain/repo_merge.go | 2 +- domain/repo_service.go | 2 +- domain/search_service.go | 2 +- domain/team_service.go | 8 +- fe/bench_test.go | 5 +- fe/mount.go | 2 +- fe/race_off_test.go | 7 ++ fe/race_on_test.go | 9 +++ fe/render/manifest.go | 2 +- fe/view/issues.go | 26 +++---- fe/web/issues/prefill.go | 2 +- fe/web/issues/show.go | 22 +++--- fe/web/notifications/handlers_test.go | 12 +-- fe/web/repo/handlers_test.go | 3 +- fe/webmw/gzip_test.go | 2 +- git/archive.go | 2 +- git/repo_blame.go | 18 ++--- git/repo_merge.go | 15 ++-- git/repo_read.go | 2 +- git/repo_refs.go | 2 +- git/repo_write.go | 8 +- git/repo_write_file.go | 1 - git/repo_write_objects.go | 23 +++--- markup/cache_test.go | 2 +- presenter/gqlissue.go | 28 +++---- presenter/gqlmodel/issue.go | 2 +- presenter/gqlmodel/repository.go | 82 ++++++++++----------- presenter/gqlmodel/user.go | 30 ++++---- presenter/gqlpull.go | 2 +- presenter/gqlreaction.go | 2 +- presenter/gqlrepository.go | 74 +++++++++---------- presenter/notification.go | 2 +- presenter/restmodel/gist.go | 38 +++++----- presenter/restmodel/search.go | 12 +-- store/queries_github_app.go | 10 +-- store/queries_keys.go | 2 +- store/queries_notifications.go | 2 +- store/queries_repo.go | 10 +-- store/store_test.go | 2 +- 70 files changed, 327 insertions(+), 415 deletions(-) create mode 100644 .DS_Store create mode 100644 fe/race_off_test.go create mode 100644 fe/race_on_test.go diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..29e1f60b6034fbafe76cc6796eb3dc3ed517b9bc GIT binary patch literal 10244 zcmeHMO=w(I6h1FA&W}cHu?rUlUPUk#F=-Vlxab(76hSFXTU`jvWF|?+%paKK^LBL?|tu`bKjdGiwarX7v8(c zJKsIuch9}|ynAl|z#Hq7_X79;ki(nG9fSBtxX4a->>4m{%2;|>AK(OZV5&O1ccRpY zq1_7?0T%%m0T%%m0T+So5CQyV)0MkRm+t8z;3D865JrIVgO4|r1ygo)sZs|%)D{5C zG`?0B{*LeU(%Z`>;Q_Nzvw zk*`Co7mVIR{~%*%~Y6rxH<8&mq#2filLW1amMA^HR;7&)9aZ z<(I1$g0U}ra3#GkQio4E(nd#1YB-wVHqQUecaI!@FBoUK4K2={WEZ^pYpJ!@a28ac zF1WMT-$HI-=lu5_JJ>Swz`{f$bG`Jz(2CY5FhWOuQMZ2@XJOGEo4byx_8S}L{~LDP zy`ghY8v9Pw^9=oAJ`R&V1hE8@IwVOFIX*A@!k(=+|BT+=&me;MR~ zp1!%tt>A@jSSQ^%cp0@)rYhrzoPimschAKn4A(jG)xE*Ei236B`r)K5HBK^^t&14H zIs95Qg(mN88#M_#*K=g}-HrMJ0zVxGq+weYTEcCfR1>dgJgD6a3wt#HnhKFc4ETz{m{m%gF(k+&jL;iQ_E zRjW6>SF=iA48`^ReEG3;v;N8}b;2Ll*B>eLc}ibS`Kv086hrH~H1-*)i($C7Hx|Oa zO~6Gx+Ycvn1?z_vj=FgqaZT)1OV!RLRF%a(X9%vTz8GqXp&~`SQBseOWH=uwI+Pho z)*2EX_|vv?Eh9gkfAFPX&NCmp*Lq>34wawoKpky0lH}Jcf10|JS-FO@wbi`IO#%_c z;`EJhmBA2#eDYe$(VVSu&z72+qQEpOP{YThy9}Grt-m+^uh=outGZAnLv-Ox;I&1gwp2V9j!gY12s^Ehl^j;mu;~#V!KWyHq z#fka7V9Ks8RduYVs$;27^Z)mM288F0N}AjM_c<%z>!Pjf=lL3GD|oqHTBl(V+p~+6 MB)I+m?b!eS2Vay5YybcN literal 0 HcmV?d00001 diff --git a/.golangci.yml b/.golangci.yml index 16af52e..0a79e00 100644 --- a/.golangci.yml +++ b/.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: diff --git a/api/graphql/connection.go b/api/graphql/connection.go index 7a663ba..8c37d02 100644 --- a/api/graphql/connection.go +++ b/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" ) diff --git a/api/graphql/errors.go b/api/graphql/errors.go index a4de89e..9b78747 100644 --- a/api/graphql/errors.go +++ b/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) diff --git a/api/graphql/helpers.go b/api/graphql/helpers.go index 4ca48b3..e0a64f8 100644 --- a/api/graphql/helpers.go +++ b/api/graphql/helpers.go @@ -8,8 +8,8 @@ package graphql import ( "context" "errors" - "sort" "regexp" + "sort" "strconv" "strings" diff --git a/api/graphql/loaders_test.go b/api/graphql/loaders_test.go index 9eea6b2..a6c63f2 100644 --- a/api/graphql/loaders_test.go +++ b/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)) diff --git a/api/graphql/nodecount.go b/api/graphql/nodecount.go index 9ef6ef4..9c93878 100644 --- a/api/graphql/nodecount.go +++ b/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} } diff --git a/api/graphql/pull.resolvers.go b/api/graphql/pull.resolvers.go index a0f9ee3..d88f513 100644 --- a/api/graphql/pull.resolvers.go +++ b/api/graphql/pull.resolvers.go @@ -483,11 +483,11 @@ func (r *pullRequestResolver) MergeCommit(ctx context.Context, obj *gqlmodel.Pul if !obj.Merged || obj.MergeCommitOID == "" { return nil, nil } - repo, err := r.Resolver.Repos.GetRepo(ctx, viewerID(ctx), obj.RepoOwner, obj.RepoName) + repo, err := r.Repos.GetRepo(ctx, viewerID(ctx), obj.RepoOwner, obj.RepoName) if err != nil { return nil, mapErr(err) } - c, err := r.Resolver.Repos.GetCommit(repo, obj.MergeCommitOID) + c, err := r.Repos.GetCommit(repo, obj.MergeCommitOID) if err != nil { return nil, nil } @@ -514,7 +514,7 @@ func (r *pullRequestResolver) ClosingIssuesReferences(ctx context.Context, obj * viewer := viewerID(ctx) issues := make([]*domain.Issue, 0) for _, n := range closingIssueNumbers(obj.Body) { - iss, err := r.Resolver.Issues.GetIssue(ctx, viewer, obj.RepoOwner, obj.RepoName, n) + iss, err := r.Issues.GetIssue(ctx, viewer, obj.RepoOwner, obj.RepoName, n) if err != nil { continue } diff --git a/api/graphql/ratelimit.go b/api/graphql/ratelimit.go index 8fc78b6..e126854 100644 --- a/api/graphql/ratelimit.go +++ b/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 diff --git a/api/graphql/ref.resolvers.go b/api/graphql/ref.resolvers.go index 5030445..d46e89b 100644 --- a/api/graphql/ref.resolvers.go +++ b/api/graphql/ref.resolvers.go @@ -77,7 +77,7 @@ func (r *mutationResolver) UpdateRef(ctx context.Context, input generated.Update shortName = ref.Name[i+1:] } return &generated.UpdateRefPayload{ - Ref: presenter.GQLRef(repo.ID, ref.Name, shortName, string(ref.Target)), + Ref: presenter.GQLRef(repo.ID, ref.Name, shortName, ref.Target), ClientMutationID: input.ClientMutationID, }, nil } diff --git a/api/graphql/server.go b/api/graphql/server.go index 9174efa..24dbd64 100644 --- a/api/graphql/server.go +++ b/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) } diff --git a/api/rest/app_contract_test.go b/api/rest/app_contract_test.go index 233e81c..0d9f4d3 100644 --- a/api/rest/app_contract_test.go +++ b/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) diff --git a/api/rest/archives_contract_test.go b/api/rest/archives_contract_test.go index a265a3b..3982835 100644 --- a/api/rest/archives_contract_test.go +++ b/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 { diff --git a/api/rest/contents_write.go b/api/rest/contents_write.go index d23822b..fe81707 100644 --- a/api/rest/contents_write.go +++ b/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 diff --git a/api/rest/git_objects_write.go b/api/rest/git_objects_write.go index 8eaeacd..3a2eb02 100644 --- a/api/rest/git_objects_write.go +++ b/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"` diff --git a/api/rest/issues.go b/api/rest/issues.go index 746d91d..a652862 100644 --- a/api/rest/issues.go +++ b/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 -} diff --git a/api/rest/issues_extra.go b/api/rest/issues_extra.go index ca7791e..68fc0a2 100644 --- a/api/rest/issues_extra.go +++ b/api/rest/issues_extra.go @@ -230,36 +230,6 @@ func handleIssueLabelRemove(d Deps) mizu.Handler { } } -// handleIssueLabelsRemoveAll serves DELETE /repos/{owner}/{repo}/issues/{number}/labels. -func handleIssueLabelsRemoveAll(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - ctx := c.Request().Context() - actor := auth.ActorFrom(ctx) - if !actor.IsUser() { - writeError(c.Writer(), errRequiresAuth()) - return nil - } - number, ok := pathInt64(c, "number") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - empty := []string{} - _, err := d.Issues.EditIssue(ctx, actor.UserID, c.Param("owner"), c.Param("repo"), number, domain.IssuePatch{ - Labels: &empty, - }) - if errors.Is(err, domain.ErrNotFound) || errors.Is(err, domain.ErrRepoNotFound) { - writeError(c.Writer(), errNotFound()) - return nil - } - if err != nil { - return err - } - c.Writer().WriteHeader(http.StatusNoContent) - return nil - } -} - // handleAssigneesList serves GET /repos/{owner}/{repo}/assignees. // Returns a list of users who can be assigned to issues in the repo // (the owner plus collaborators). diff --git a/api/rest/keys.go b/api/rest/keys.go index 2a47afb..52e7f98 100644 --- a/api/rest/keys.go +++ b/api/rest/keys.go @@ -287,4 +287,3 @@ func userKeyToJSON(k *store.SSHKeyRow, d Deps) map[string]any { "read_only": false, } } - diff --git a/api/rest/misc_compat.go b/api/rest/misc_compat.go index fb7e55c..376bfe2 100644 --- a/api/rest/misc_compat.go +++ b/api/rest/misc_compat.go @@ -433,7 +433,7 @@ func handleSingleCommitGet(d Deps) mizu.Handler { } // handleRepoTeamsList serves GET /repos/{owner}/{repo}/teams. -func handleRepoTeamsList(d Deps) mizu.Handler { +func handleRepoTeamsList(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, []any{}) return nil @@ -592,7 +592,7 @@ func handleSearchUsers(d Deps) mizu.Handler { } // handleSearchCommits serves GET /search/commits. -func handleSearchCommits(d Deps) mizu.Handler { +func handleSearchCommits(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, map[string]any{ "total_count": 0, @@ -604,7 +604,7 @@ func handleSearchCommits(d Deps) mizu.Handler { } // handleSearchTopics serves GET /search/topics. -func handleSearchTopics(d Deps) mizu.Handler { +func handleSearchTopics(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, map[string]any{ "total_count": 0, @@ -616,7 +616,7 @@ func handleSearchTopics(d Deps) mizu.Handler { } // handleSearchLabels serves GET /search/labels. -func handleSearchLabels(d Deps) mizu.Handler { +func handleSearchLabels(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, map[string]any{ "total_count": 0, @@ -668,7 +668,7 @@ func handleOrgTeamsList(d Deps) mizu.Handler { } // handleOrgTeamReposList serves GET /orgs/{org}/teams/{team_slug}/repos. -func handleOrgTeamReposList(d Deps) mizu.Handler { +func handleOrgTeamReposList(_ Deps) mizu.Handler { return func(c *mizu.Ctx) error { writeJSON(c.Writer(), http.StatusOK, []any{}) return nil @@ -816,9 +816,9 @@ func handleInstallationRepos(d Deps) mizu.Handler { items = append(items, d.URLs.Repository(r, d.NodeFormat, nil)) } writeJSON(c.Writer(), http.StatusOK, map[string]any{ - "total_count": len(items), - "repository_selection": inst.RepositorySelection, - "repositories": items, + "total_count": len(items), + "repository_selection": inst.RepositorySelection, + "repositories": items, }) return nil } diff --git a/api/rest/pulls_extra.go b/api/rest/pulls_extra.go index ad712ff..13cf966 100644 --- a/api/rest/pulls_extra.go +++ b/api/rest/pulls_extra.go @@ -99,37 +99,6 @@ func handleReviewCommentEdit(d Deps) mizu.Handler { } } -// handleReviewCommentDelete serves DELETE /repos/{owner}/{repo}/pulls/comments/{comment_id}. -func handleReviewCommentDelete(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - ctx := c.Request().Context() - actor := auth.ActorFrom(ctx) - if !actor.IsUser() { - writeError(c.Writer(), errRequiresAuth()) - return nil - } - id, ok := pathInt64(c, "comment_id") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - err := d.Reviews.DeleteReviewComment(ctx, actor.UserID, id) - if errors.Is(err, domain.ErrNotFound) { - writeError(c.Writer(), errNotFound()) - return nil - } - if errors.Is(err, domain.ErrForbidden) { - writeError(c.Writer(), errForbidden("not allowed")) - return nil - } - if err != nil { - return err - } - c.Writer().WriteHeader(http.StatusNoContent) - return nil - } -} - // handleAllReviewCommentsList serves GET /repos/{owner}/{repo}/pulls/comments. func handleAllReviewCommentsList(d Deps) mizu.Handler { return func(c *mizu.Ctx) error { @@ -189,40 +158,6 @@ func handleRequestedReviewersAdd(d Deps) mizu.Handler { } } -// handleRequestedReviewersRemove serves DELETE /repos/{owner}/{repo}/pulls/{number}/requested_reviewers. -func handleRequestedReviewersRemove(d Deps) mizu.Handler { - return func(c *mizu.Ctx) error { - number, ok := pathInt64(c, "number") - if !ok { - writeError(c.Writer(), errNotFound()) - return nil - } - ctx := c.Request().Context() - actor := auth.ActorFrom(ctx) - if !actor.IsUser() { - writeError(c.Writer(), errRequiresAuth()) - return nil - } - var body struct { - Reviewers []string `json:"reviewers"` - TeamReviewers []string `json:"team_reviewers"` - } - if !decodeJSON(c, &body) { - return nil - } - owner, repo := c.Param("owner"), c.Param("repo") - pr, err := d.Pulls.RemoveRequestedReviewers(ctx, actor.UserID, owner, repo, number, body.Reviewers) - if pullError(c.Writer(), err) { - return nil - } - if err != nil { - return err - } - writeJSON(c.Writer(), http.StatusOK, d.URLs.PullRequest(owner, repo, pr, d.NodeFormat, true)) - return nil - } -} - // handleRequestedReviewersList serves GET /repos/{owner}/{repo}/pulls/{number}/requested_reviewers. func handleRequestedReviewersList(d Deps) mizu.Handler { return func(c *mizu.Ctx) error { diff --git a/api/rest/repo_permissions_contract_test.go b/api/rest/repo_permissions_contract_test.go index 387d483..fc478ef 100644 --- a/api/rest/repo_permissions_contract_test.go +++ b/api/rest/repo_permissions_contract_test.go @@ -111,7 +111,7 @@ func TestRepoPermissionsCollaborator(t *testing.T) { } // The repo list renders the same resolved block per item. - resp, body = authedGet(t, fx.srv, "/users/octocat/repos", "token "+hubToken) + resp, _ = authedGet(t, fx.srv, "/users/octocat/repos", "token "+hubToken) if resp.StatusCode != http.StatusOK { t.Fatalf("list: status %d", resp.StatusCode) } diff --git a/api/rest/repo_surface_contract_test.go b/api/rest/repo_surface_contract_test.go index c274d3f..6fc0231 100644 --- a/api/rest/repo_surface_contract_test.go +++ b/api/rest/repo_surface_contract_test.go @@ -109,7 +109,9 @@ func TestCommunityProfile(t *testing.T) { HealthPercentage int `json:"health_percentage"` Description *string `json:"description"` Files struct { - Readme *struct{ HTMLURL string `json:"html_url"` } `json:"readme"` + Readme *struct { + HTMLURL string `json:"html_url"` + } `json:"readme"` License *struct{} `json:"license"` Contributing *struct{} `json:"contributing"` } `json:"files"` diff --git a/api/rest/root.go b/api/rest/root.go index 44f9b77..10c3513 100644 --- a/api/rest/root.go +++ b/api/rest/root.go @@ -8,7 +8,7 @@ import ( // apiRoot is GitHub's root hypermedia document: the *_url template map clients // like Octokit's hypermedia mode read to derive every other endpoint instead of -// hardcoding paths. Field order matches api.github.com. +// hardcoding paths. Field order matches the GitHub REST API root. type apiRoot struct { CurrentUserURL string `json:"current_user_url"` CurrentUserAuthorizationsHTMLURL string `json:"current_user_authorizations_html_url"` @@ -45,8 +45,8 @@ type apiRoot struct { } // handleAPIRoot serves GET at the API root. The body is the same template map -// api.github.com returns, with every URL rebuilt on the configured hosts. It is -// computed once at mount time: nothing in it varies per request. +// the GitHub REST API root returns, with every URL rebuilt on the configured +// hosts. It is computed once at mount time: nothing in it varies per request. func handleAPIRoot(d Deps) mizu.Handler { api := d.URLs.APIBase() doc := apiRoot{ diff --git a/api/rest/social_contract_test.go b/api/rest/social_contract_test.go index cf1d0c5..8281b0b 100644 --- a/api/rest/social_contract_test.go +++ b/api/rest/social_contract_test.go @@ -188,7 +188,7 @@ func TestWatchingFlow(t *testing.T) { if sub["ignored"] != true { t.Fatalf("ignored subscription = %v", sub) } - resp, body = authedGet(t, fx.srv, "/repos/octocat/hello/subscribers", "token "+fx.token) + _, body = authedGet(t, fx.srv, "/repos/octocat/hello/subscribers", "token "+fx.token) subs = decodeArray(t, body) if len(subs) != 0 { t.Fatalf("subscribers after ignore = %v, want []", subs) @@ -239,12 +239,12 @@ func TestFollowingFlow(t *testing.T) { } // octocat's following list shows hubber; hubber's followers show octocat. - resp, body = authedGet(t, fx.srv, "/user/following", "token "+octo) + _, body = authedGet(t, fx.srv, "/user/following", "token "+octo) following := decodeArray(t, body) if len(following) != 1 || following[0]["login"] != "hubber" { t.Fatalf("following = %v, want [hubber]", following) } - resp, body = authedGet(t, fx.srv, "/users/hubber/followers", "token "+hubber) + _, body = authedGet(t, fx.srv, "/users/hubber/followers", "token "+hubber) followers := decodeArray(t, body) if len(followers) != 1 || followers[0]["login"] != "octocat" { t.Fatalf("followers = %v, want [octocat]", followers) diff --git a/auth/app_jwt.go b/auth/app_jwt.go index b60a144..8e81fd3 100644 --- a/auth/app_jwt.go +++ b/auth/app_jwt.go @@ -40,7 +40,7 @@ func (c *publicKeyCache) set(pk int64, k *rsa.PublicKey) { // On success it returns a KindAppJWT actor whose AppID is the app's internal PK. func (s *Service) resolveAppJWT(ctx context.Context, raw string) (*Actor, error) { var claims jwt.RegisteredClaims - keyFunc := func(t *jwt.Token) (any, error) { + keyFunc := func(_ *jwt.Token) (any, error) { iss, err := claims.GetIssuer() if err != nil || iss == "" { return nil, ErrBadCredentials @@ -130,8 +130,7 @@ func (s *Service) resolveInstallation(ctx context.Context, raw string) (*Actor, // and own the installation. On success it returns the plaintext token and expiry. // repos and permissions narrow the grant (empty = full installation grant). func (s *Service) CreateInstallationToken(ctx context.Context, actor *Actor, instPK int64, - repos []string, permissions map[string]string) (plaintext string, expiresAt time.Time, err error) { - + _ []string, _ map[string]string) (plaintext string, expiresAt time.Time, err error) { if actor.Kind != KindAppJWT { return "", time.Time{}, ErrBadCredentials } diff --git a/auth/webflow.go b/auth/webflow.go index 087ddac..be359dc 100644 --- a/auth/webflow.go +++ b/auth/webflow.go @@ -45,8 +45,8 @@ func redirectAllowed(app *store.OAuthAppRow, redirectURI string) bool { return strings.HasPrefix(redirectURI, app.CallbackURL) } -// AuthCodeRequest holds the parameters the caller passes to GenerateAuthCode. -type AuthCodeRequest struct { +// CodeRequest holds the parameters the caller passes to GenerateAuthCode. +type CodeRequest struct { ClientID string RedirectURI string Scope string @@ -56,7 +56,7 @@ type AuthCodeRequest struct { // GenerateAuthCode creates a new single-use authorization code for the OAuth // web flow. It returns the opaque plaintext code the server embeds in the // redirect to the client's redirect_uri. -func (s *Service) GenerateAuthCode(ctx context.Context, req AuthCodeRequest) (string, error) { +func (s *Service) GenerateAuthCode(ctx context.Context, req CodeRequest) (string, error) { return s.GenerateOAuthAuthCode(ctx, req.ClientID, req.RedirectURI, req.Scope, req.UserPK) } diff --git a/conform/conform.go b/conform/conform.go index 927bb9d..19fa47a 100644 --- a/conform/conform.go +++ b/conform/conform.go @@ -54,8 +54,11 @@ func Run(opts Options) *Report { type Status int const ( + // Pass means the check ran and the behavior matched. Pass Status = iota + // Fail means the check ran and the behavior did not match. Fail + // Skip means the check did not run. Skip ) diff --git a/domain/event.go b/domain/event.go index 1cbce8c..6e9b725 100644 --- a/domain/event.go +++ b/domain/event.go @@ -301,7 +301,7 @@ func (s *EventService) OrgFeed(ctx context.Context, viewerPK int64, org string, // following anyone. The publicOnly flag is accepted for the // /received_events/public twin and is a no-op while the feed is already public. // It backs GET /users/{u}/received_events[/public]. -func (s *EventService) ReceivedFeed(ctx context.Context, login string, perPage int, publicOnly bool) ([]Event, error) { +func (s *EventService) ReceivedFeed(ctx context.Context, login string, perPage int, _ bool) ([]Event, error) { if _, err := s.store.UserByLogin(ctx, login); err != nil { return nil, ErrUserNotFound } diff --git a/domain/key_service.go b/domain/key_service.go index 485aa68..a500a7b 100644 --- a/domain/key_service.go +++ b/domain/key_service.go @@ -71,7 +71,6 @@ func (s *KeyService) GetDeployKey(ctx context.Context, repoPK, dbID int64) (*sto // CreateDeployKey parses and registers a new deploy key on repoPK. func (s *KeyService) CreateDeployKey(ctx context.Context, repoPK, ownerPK int64, title, rawKey string, readOnly bool) (*store.SSHKeyRow, error) { - keyType, fp, err := parseSSHPublicKey(rawKey) if err != nil { return nil, ErrInvalidSSHKey diff --git a/domain/push.go b/domain/push.go index 11009cf..0aeae45 100644 --- a/domain/push.go +++ b/domain/push.go @@ -121,8 +121,8 @@ func (s *RepoService) SyntheticHeadPush(ctx context.Context, repoPK int64, defau Protocol: "http", Updates: []RefUpdate{{ Ref: ref, - OldSHA: string(sha), - NewSHA: string(sha), + OldSHA: sha, + NewSHA: sha, }}, }, true, nil } diff --git a/domain/repo_community.go b/domain/repo_community.go index 0edbec3..98ddf85 100644 --- a/domain/repo_community.go +++ b/domain/repo_community.go @@ -91,7 +91,7 @@ func (s *RepoService) repoFilePaths(repo *Repo) ([]string, error) { // is the one GitHub would surface. var ( communityReadme = matcher("README") - communityLicense = prefixMatcher([]string{""}, "LICENSE", "LICENCE", "COPYING") + communityLicense = prefixMatcher([]string{""}, "LICENSE", "LICENSE", "COPYING") communityContributing = matcher("CONTRIBUTING") communityCodeOfConduct = matcher("CODE_OF_CONDUCT") communityIssueTemplate = matcher("ISSUE_TEMPLATE") diff --git a/domain/repo_merge.go b/domain/repo_merge.go index d963764..a119a48 100644 --- a/domain/repo_merge.go +++ b/domain/repo_merge.go @@ -74,7 +74,7 @@ func (s *RepoService) MergeBranch(ctx context.Context, actorPK int64, owner, nam // Fan the merge out as a push to the base branch so the activity feed and a // repository's webhooks observe it the same as any other update to that // branch. Delivery is best-effort, exactly as a real push's is. - s.OnPush(ctx, PushBatch{ + _ = s.OnPush(ctx, PushBatch{ RepoPK: repo.PK, PusherPK: actorPK, Protocol: "api", diff --git a/domain/repo_service.go b/domain/repo_service.go index 9910e76..6805c2f 100644 --- a/domain/repo_service.go +++ b/domain/repo_service.go @@ -959,7 +959,7 @@ func checkCurrentBlob(gr *git.Repo, in WriteFileInput) error { ref = "HEAD" } cur, err := gr.PathAt(ref, in.Path) - if err != nil || cur.IsDir || string(cur.Entry.SHA) != in.CurrentBlobSHA { + if err != nil || cur.IsDir || cur.Entry.SHA != in.CurrentBlobSHA { return ErrConflict } return nil diff --git a/domain/search_service.go b/domain/search_service.go index 46245b4..f904ffd 100644 --- a/domain/search_service.go +++ b/domain/search_service.go @@ -319,7 +319,7 @@ func (s *SearchService) codeIndexHeadSHA(repo *Repo) string { if err != nil { return "" } - return string(br.Commit) + return br.Commit } // codeIndexFresh reports whether the stored index state already matches head. diff --git a/domain/team_service.go b/domain/team_service.go index 93bfc9b..a73a093 100644 --- a/domain/team_service.go +++ b/domain/team_service.go @@ -74,10 +74,10 @@ func (s *TeamService) CreateTeam(ctx context.Context, orgPK int64, name, descrip permission = "pull" } t := &store.TeamRow{ - OrgPK: orgPK, - Name: name, - Slug: slugify(name), - Privacy: privacy, + OrgPK: orgPK, + Name: name, + Slug: slugify(name), + Privacy: privacy, Permission: permission, } if d := strings.TrimSpace(description); d != "" { diff --git a/fe/bench_test.go b/fe/bench_test.go index 1382ebc..e77b086 100644 --- a/fe/bench_test.go +++ b/fe/bench_test.go @@ -576,7 +576,10 @@ func TestPageSLO(t *testing.T) { } avg := time.Since(start) / measure t.Logf("%s: avg=%v", p.name, avg) - if avg > budget { + // Under -race the instrumentation inflates every request far past the + // budget; the requests above still run for the race detector, but the + // wall-clock gate only holds on an uninstrumented build. + if !raceEnabled && avg > budget { t.Errorf("%s: avg=%v exceeds the %v budget", p.name, avg, budget) } }) diff --git a/fe/mount.go b/fe/mount.go index a1ae81b..5a43dda 100644 --- a/fe/mount.go +++ b/fe/mount.go @@ -604,7 +604,7 @@ func mountProfile(page *mizu.Router, d Deps) { // absence. The redirect itself needs no service, so it is always registered: the // profile it points at renders or 404s on the user service's own gate. "orgs" is // a reserved top-level name (fe/route), so it is never read as a /{owner} login. -func mountOrgs(page *mizu.Router, d Deps) { +func mountOrgs(page *mizu.Router, _ Deps) { page.Get("/orgs/{org}", func(c *mizu.Ctx) error { return c.Redirect(http.StatusFound, route.Profile(c.Param("org"))) }) diff --git a/fe/race_off_test.go b/fe/race_off_test.go new file mode 100644 index 0000000..e6093ea --- /dev/null +++ b/fe/race_off_test.go @@ -0,0 +1,7 @@ +//go:build !race + +package fe_test + +// raceEnabled is false in a normal (non -race) build, so the SLO gate enforces +// its wall-clock budget. +const raceEnabled = false diff --git a/fe/race_on_test.go b/fe/race_on_test.go new file mode 100644 index 0000000..4d9eb6d --- /dev/null +++ b/fe/race_on_test.go @@ -0,0 +1,9 @@ +//go:build race + +package fe_test + +// raceEnabled is true when the test binary is built with -race. The SLO gate +// reads it to skip its wall-clock budget assertion, since race instrumentation +// inflates every request well past the budget while the requests themselves +// still exercise the handlers for the race detector. +const raceEnabled = true diff --git a/fe/render/manifest.go b/fe/render/manifest.go index 2aee1df..312cc73 100644 --- a/fe/render/manifest.go +++ b/fe/render/manifest.go @@ -87,7 +87,7 @@ func (s *Set) AssetHandler() mizu.Handler { if err != nil { return s.NotFound(c) } - defer f.Close() + defer func() { _ = f.Close() }() h := c.Header() h.Set("Content-Type", contentTypeFor(clean)) diff --git a/fe/view/issues.go b/fe/view/issues.go index 110165e..0ec7135 100644 --- a/fe/view/issues.go +++ b/fe/view/issues.go @@ -174,19 +174,19 @@ type ComposerVM struct { // IssueDetailVM is the issue detail page: the title and state header, the comment // timeline, the sidebar, and the composer. type IssueDetailVM struct { - Chrome Chrome - Header RepoHeaderVM - Nav TreeNav - Repo RepoRef - Number int64 - Title string - State IssueStateVM - Author UserChipVM - OpenedAt string - OpenedISO string - Locked bool - EditURL string // the edit-title form target, shown when the viewer can edit - CanEdit bool + Chrome Chrome + Header RepoHeaderVM + Nav TreeNav + Repo RepoRef + Number int64 + Title string + State IssueStateVM + Author UserChipVM + OpenedAt string + OpenedISO string + Locked bool + EditURL string // the edit-title form target, shown when the viewer can edit + CanEdit bool Timeline []CommentVM TimelinePager Pager // prev/next over a long comment thread; zero value hides it Sidebar SidebarVM diff --git a/fe/web/issues/prefill.go b/fe/web/issues/prefill.go index c9accf5..9c0eda0 100644 --- a/fe/web/issues/prefill.go +++ b/fe/web/issues/prefill.go @@ -44,7 +44,7 @@ func (h *Handlers) prefillNewIssue(c *mizu.Ctx, repo *domain.Repo, vm *view.NewI // and folds it into the form: the body is the template text with its YAML // front matter stripped, and the front matter's title, labels, and assignees // fill any field the query left empty. -func (h *Handlers) applyIssueTemplate(ctx context.Context, repo *domain.Repo, name string, vm *view.NewIssueVM) { +func (h *Handlers) applyIssueTemplate(_ context.Context, repo *domain.Repo, name string, vm *view.NewIssueVM) { // The template is one file name inside the template directory, never a // path of its own. if name != path.Base(name) || strings.HasPrefix(name, ".") { diff --git a/fe/web/issues/show.go b/fe/web/issues/show.go index fef7f0d..3c39e13 100644 --- a/fe/web/issues/show.go +++ b/fe/web/issues/show.go @@ -97,17 +97,17 @@ func (h *Handlers) detail(ctx context.Context, c *mizu.Ctx, repo *domain.Repo, i open := iss.State == "open" vm := view.IssueDetailVM{ - Chrome: h.chrome(c, iss.Title+" #"+strconv.FormatInt(iss.Number, 10)), - Header: h.header(c.Context(), repo), - Nav: h.nav(repo), - Repo: repoRef(repo), - Number: iss.Number, - Title: iss.Title, - State: stateBadge(iss), - Author: h.userChip(iss.User), - OpenedAt: iss.CreatedAt.UTC().Format("Jan 2, 2006"), - OpenedISO: iss.CreatedAt.UTC().Format(time.RFC3339), - Locked: iss.Locked, + Chrome: h.chrome(c, iss.Title+" #"+strconv.FormatInt(iss.Number, 10)), + Header: h.header(c.Context(), repo), + Nav: h.nav(repo), + Repo: repoRef(repo), + Number: iss.Number, + Title: iss.Title, + State: stateBadge(iss), + Author: h.userChip(iss.User), + OpenedAt: iss.CreatedAt.UTC().Format("Jan 2, 2006"), + OpenedISO: iss.CreatedAt.UTC().Format(time.RFC3339), + Locked: iss.Locked, CanEdit: write, FormError: formError, TimelinePager: pager, diff --git a/fe/web/notifications/handlers_test.go b/fe/web/notifications/handlers_test.go index 0f6cbbe..40ffe69 100644 --- a/fe/web/notifications/handlers_test.go +++ b/fe/web/notifications/handlers_test.go @@ -60,13 +60,13 @@ func newFixture(t *testing.T) fixture { // Two issue rows: a plain issue and a pull request sharing the number // sequence. Each backs one notification thread. - issuePK := seedIssue(t, ctx, st, hello.PK, "the unread bug", false) - pullPK := seedIssue(t, ctx, st, hello.PK, "the read pull", true) + issuePK := seedIssue(ctx, t, st, hello.PK, "the unread bug", false) + pullPK := seedIssue(ctx, t, st, hello.PK, "the read pull", true) // The unread thread shows in both the Inbox and the All filter; the read one // only in All. - upsertThread(t, ctx, st, octocat.PK, hello.PK, issuePK, "mention") - readPK := upsertThread(t, ctx, st, octocat.PK, hello.PK, pullPK, "review_requested") + upsertThread(ctx, t, st, octocat.PK, hello.PK, issuePK, "mention") + readPK := upsertThread(ctx, t, st, octocat.PK, hello.PK, pullPK, "review_requested") if err := st.MarkNotificationThreadRead(ctx, readPK); err != nil { t.Fatalf("mark read: %v", err) } @@ -123,7 +123,7 @@ func newFixture(t *testing.T) fixture { // number from the repo's sequence, and returns its PK. The domain gates // creation on access this fixture does not grant, and the inbox only needs the // row to resolve a thread's subject. -func seedIssue(t *testing.T, ctx context.Context, st *store.Store, repoPK int64, title string, isPull bool) int64 { +func seedIssue(ctx context.Context, t *testing.T, st *store.Store, repoPK int64, title string, isPull bool) int64 { t.Helper() var pk int64 if err := st.WithTx(ctx, func(tx *store.Tx) error { @@ -145,7 +145,7 @@ func seedIssue(t *testing.T, ctx context.Context, st *store.Store, repoPK int64, // upsertThread records a notification thread for the user on the issue and // returns its PK. -func upsertThread(t *testing.T, ctx context.Context, st *store.Store, userPK, repoPK, issuePK int64, reason string) int64 { +func upsertThread(ctx context.Context, t *testing.T, st *store.Store, userPK, repoPK, issuePK int64, reason string) int64 { t.Helper() row := &store.NotificationThreadRow{UserPK: userPK, RepoPK: repoPK, IssuePK: issuePK, Reason: reason} if err := st.UpsertNotificationThread(ctx, row); err != nil { diff --git a/fe/web/repo/handlers_test.go b/fe/web/repo/handlers_test.go index d3f28e9..5fb9143 100644 --- a/fe/web/repo/handlers_test.go +++ b/fe/web/repo/handlers_test.go @@ -3,7 +3,6 @@ package repo import ( "bytes" "context" - "github.com/tamnd/githome/fe/route" "io" "log/slog" "net/http" @@ -16,6 +15,8 @@ import ( "testing" "time" + "github.com/tamnd/githome/fe/route" + gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" diff --git a/fe/webmw/gzip_test.go b/fe/webmw/gzip_test.go index 6054a75..2d7d336 100644 --- a/fe/webmw/gzip_test.go +++ b/fe/webmw/gzip_test.go @@ -12,7 +12,7 @@ import ( // gzipEcho serves a fixed body under the given content type, optionally with a // preset Content-Encoding, behind the Gzip layer. func gzipEcho(contentType, encoding, body string) http.Handler { - return Gzip(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + return Gzip(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", contentType) if encoding != "" { w.Header().Set("Content-Encoding", encoding) diff --git a/git/archive.go b/git/archive.go index 93c4ee7..e8dad06 100644 --- a/git/archive.go +++ b/git/archive.go @@ -25,7 +25,7 @@ const archiveTimeout = 5 * time.Minute func (s *Store) ArchiveStream(ctx context.Context, pk int64, format, prefix string, sha SHA, w io.Writer) error { ctx, cancel := context.WithTimeout(ctx, archiveTimeout) defer cancel() - args := []string{"archive", "--format=" + format, "--prefix=" + prefix + "/", "--end-of-options", string(sha)} + args := []string{"archive", "--format=" + format, "--prefix=" + prefix + "/", "--end-of-options", sha} full := append([]string{"--git-dir", s.runDir(pk)}, args...) cmd := exec.CommandContext(ctx, s.bin(), full...) cmd.Env = baseEnv() diff --git a/git/repo_blame.go b/git/repo_blame.go index 1168dc1..97084fc 100644 --- a/git/repo_blame.go +++ b/git/repo_blame.go @@ -49,40 +49,40 @@ func blameCost(lines []BlameLine) int64 { // handled is false when the Repo has no store or the run failed, in which case // the caller falls back to go-git; when handled is true the result, including // a size-cap refusal, is final. -func (r *Repo) blameBatch(ref, path string) (lines []BlameLine, err error, handled bool) { +func (r *Repo) blameBatch(ref, path string) (lines []BlameLine, handled bool, err error) { if r.store == nil { - return nil, nil, false + return nil, false, nil } c, cerr := r.commitFromRev(ref) if cerr != nil { - return nil, nil, false + return nil, false, nil } sha := c.Hash.String() key := strconv.FormatInt(r.pk, 10) + ":" + sha + ":" + path if v, ok := r.store.blames.get(key); ok { - return v, nil, true + return v, true, nil } if r.maxBlobBytes > 0 { res, rerr := r.store.run(context.Background(), r.pk, nil, "cat-file", "-s", sha+":"+path) if rerr != nil || res.code != 0 { - return nil, nil, false + return nil, false, nil } if n, perr := strconv.ParseInt(strings.TrimSpace(string(res.stdout)), 10, 64); perr == nil && n > r.maxBlobBytes { - return nil, ErrBlobTooLarge, true + return nil, true, ErrBlobTooLarge } } ctx, cancel := context.WithTimeout(context.Background(), blameTimeout) defer cancel() res, rerr := r.store.run(ctx, r.pk, nil, "blame", "--porcelain", "--end-of-options", sha, "--", path) if rerr != nil || res.code != 0 { - return nil, nil, false + return nil, false, nil } lines, ok := parseBlamePorcelain(string(res.stdout)) if !ok { - return nil, nil, false + return nil, false, nil } r.store.blames.put(key, lines) - return lines, nil, true + return lines, true, nil } // parseBlamePorcelain parses git blame --porcelain output. Commit metadata diff --git a/git/repo_merge.go b/git/repo_merge.go index 74d9dbe..b00e739 100644 --- a/git/repo_merge.go +++ b/git/repo_merge.go @@ -96,13 +96,14 @@ func (s *Store) CommitsBetween(ctx context.Context, pk int64, base, head SHA) ([ return s.CommitsBetweenN(ctx, pk, base, head, 0) } -// CommitsBetweenN is CommitsBetween bounded to at most max commits (the newest -// max of the range, still returned oldest first), so a compare across thousands -// of commits never loads the whole range into memory. max <= 0 is unbounded. -func (s *Store) CommitsBetweenN(ctx context.Context, pk int64, base, head SHA, max int) ([]Commit, error) { +// CommitsBetweenN is CommitsBetween bounded to at most limit commits (the +// newest limit of the range, still returned oldest first), so a compare across +// thousands of commits never loads the whole range into memory. limit <= 0 is +// unbounded. +func (s *Store) CommitsBetweenN(ctx context.Context, pk int64, base, head SHA, limit int) ([]Commit, error) { args := []string{"log", "--reverse", "--pretty=format:" + logRecordFormat} - if max > 0 { - args = append(args, "-n", strconv.Itoa(max)) + if limit > 0 { + args = append(args, "-n", strconv.Itoa(limit)) } args = append(args, base+".."+head) r, err := s.run(ctx, pk, nil, args...) @@ -391,7 +392,7 @@ func (s *Store) DiffDirect(ctx context.Context, pk int64, base, head SHA) ([]byt // commit page's .patch form serves. format-patch -1 handles a root commit // without a parent. func (s *Store) FormatPatchCommit(ctx context.Context, pk int64, sha SHA) ([]byte, error) { - args := []string{"format-patch", "-1", "--stdout", "--no-color", "--end-of-options", string(sha)} + args := []string{"format-patch", "-1", "--stdout", "--no-color", "--end-of-options", sha} r, err := s.run(ctx, pk, nil, args...) if err != nil { return nil, err diff --git a/git/repo_read.go b/git/repo_read.go index 840ffa8..b457a4e 100644 --- a/git/repo_read.go +++ b/git/repo_read.go @@ -602,7 +602,7 @@ type BlameLine struct { // it. It returns ErrObjectNotFound when the path does not exist in the tree at // ref, and ErrRepoNotFound for other resolution failures. func (r *Repo) Blame(ref, path string) ([]BlameLine, error) { - if lines, err, handled := r.blameBatch(ref, path); handled { + if lines, handled, err := r.blameBatch(ref, path); handled { return lines, err } c, err := r.commitFromRev(ref) diff --git a/git/repo_refs.go b/git/repo_refs.go index 92c7495..2d6e3e3 100644 --- a/git/repo_refs.go +++ b/git/repo_refs.go @@ -112,7 +112,7 @@ func (r *Repo) logBatch(opts LogOpts, limit int) ([]Commit, bool) { if opts.Until != nil { args = append(args, "--until="+opts.Until.Format(time.RFC3339)) } - args = append(args, "--end-of-options", string(opts.From)) + args = append(args, "--end-of-options", opts.From) if opts.Path != "" { args = append(args, "--", opts.Path) } diff --git a/git/repo_write.go b/git/repo_write.go index bc0c395..536a174 100644 --- a/git/repo_write.go +++ b/git/repo_write.go @@ -91,7 +91,7 @@ func (s *Store) runEnv(ctx context.Context, pk int64, extraEnv []string, stdin i // ours, not a git failure. It exists for reads whose size the server cannot // know in advance, like a commit's patch, where buffering an unbounded diff // would defeat the point of capping it. -func (s *Store) runLimited(ctx context.Context, pk int64, max int64, args ...string) (stdout []byte, truncated bool, code int, err error) { +func (s *Store) runLimited(ctx context.Context, pk int64, limit int64, args ...string) (stdout []byte, truncated bool, code int, err error) { ctx, cancel := context.WithCancel(ctx) defer cancel() full := append([]string{"--git-dir", s.runDir(pk)}, args...) @@ -104,9 +104,9 @@ func (s *Store) runLimited(ctx context.Context, pk int64, max int64, args ...str if err := cmd.Start(); err != nil { return nil, false, 0, fmt.Errorf("git %s: %w", strings.Join(args, " "), err) } - out, readErr := io.ReadAll(io.LimitReader(pipe, max+1)) - if int64(len(out)) > max { - out, truncated = out[:max], true + out, readErr := io.ReadAll(io.LimitReader(pipe, limit+1)) + if int64(len(out)) > limit { + out, truncated = out[:limit], true cancel() // kill the producer; Wait's error is then ours to ignore } waitErr := cmd.Wait() diff --git a/git/repo_write_file.go b/git/repo_write_file.go index ad3e2a2..4585c49 100644 --- a/git/repo_write_file.go +++ b/git/repo_write_file.go @@ -188,7 +188,6 @@ func (r *Repo) rebuildTree( blobHash plumbing.Hash, del bool, ) (plumbing.Hash, error) { - name := parts[0] var newEntries []object.TreeEntry diff --git a/git/repo_write_objects.go b/git/repo_write_objects.go index 7fbbb62..5ddb4d8 100644 --- a/git/repo_write_objects.go +++ b/git/repo_write_objects.go @@ -11,7 +11,7 @@ import ( // CreateBlobInput holds content for POST /git/blobs. type CreateBlobInput struct { - Content []byte // already decoded (caller converts from utf-8 or base64) + Content []byte // already decoded (caller converts from utf-8 or base64) } // CreateBlobResult holds the outcome of CreateBlob. @@ -42,10 +42,10 @@ func (r *Repo) CreateBlob(in CreateBlobInput) (*CreateBlobResult, error) { // CreateTreeEntry is one entry passed to CreateTree. type CreateTreeEntry struct { Path string - Mode string // "100644", "100755", "040000", "160000", "120000" + Mode string // "100644", "100755", "040000", "160000", "120000" Type ObjectType // "blob", "tree", "commit" - SHA string // object SHA; empty string is allowed for inline blobs - Content []byte // non-nil: create an inline blob first + SHA string // object SHA; empty string is allowed for inline blobs + Content []byte // non-nil: create an inline blob first } // CreateTreeResult holds the outcome of CreateTree. @@ -132,11 +132,11 @@ func (r *Repo) CreateTree(baseTreeSHA string, entries []CreateTreeEntry) (*Creat // CreateCommitInput holds parameters for POST /git/commits. type CreateCommitInput struct { - Message string - Tree string - Parents []string - Author Signature - Committer Signature + Message string + Tree string + Parents []string + Author Signature + Committer Signature } // CreateCommitResult holds the outcome of CreateCommit. @@ -259,7 +259,9 @@ func (r *Repo) CreateTag(in CreateTagInput) (*CreateTagResult, error) { }, nil } -// GetTag reads an annotated tag object by SHA. +// GetTagResult is the decoded form of an annotated tag object: the tag's own +// SHA and name, its message, and the object it points at with that object's +// type and the tagger signature. type GetTagResult struct { SHA string Tag string @@ -269,6 +271,7 @@ type GetTagResult struct { Tagger Signature } +// GetTag reads an annotated tag object by SHA. func (r *Repo) GetTag(sha string) (*GetTagResult, error) { st := r.repo.Storer t, err := object.GetTag(st, plumbing.NewHash(sha)) diff --git a/markup/cache_test.go b/markup/cache_test.go index bf5a641..1edab28 100644 --- a/markup/cache_test.go +++ b/markup/cache_test.go @@ -60,7 +60,7 @@ func TestRenderWithResolveBypassesCache(t *testing.T) { rc := RenderContext{ Mode: ModeComment, Repo: &RepoRef{Owner: "octo", Name: "hello", ID: 7}, - Resolve: func(ctx context.Context, kind RefKind, raw string) (string, bool) { + Resolve: func(_ context.Context, _ RefKind, _ string) (string, bool) { return "", false }, } diff --git a/presenter/gqlissue.go b/presenter/gqlissue.go index 515c34a..dc9ca41 100644 --- a/presenter/gqlissue.go +++ b/presenter/gqlissue.go @@ -16,20 +16,20 @@ import ( func (b *URLBuilder) GQLIssue(owner, repo string, iss *domain.Issue, format nodeid.Format) *gqlmodel.Issue { num := strconv.FormatInt(iss.Number, 10) out := &gqlmodel.Issue{ - ID: nodeid.Encode(nodeid.KindIssue, iss.ID, format), - Number: int32(iss.Number), - Title: iss.Title, - Body: deref(iss.Body), - State: issueState(iss.State), - URL: gqlmodel.URI(b.RepoHTML(owner, repo) + "/issues/" + num), - Locked: iss.Locked, - Closed: iss.State == "closed", - Author: b.gqlActor(iss.User, format), - CreatedAt: gqlmodel.NewDateTime(iss.CreatedAt), - UpdatedAt: gqlmodel.NewDateTime(iss.UpdatedAt), - Labels: b.gqlLabelConnection(owner, repo, iss.Labels, format), - Assignees: b.GQLUserConnection(iss.Assignees, format), - Milestone: b.GQLMilestone(owner, repo, iss.Milestone, format), + ID: nodeid.Encode(nodeid.KindIssue, iss.ID, format), + Number: int32(iss.Number), + Title: iss.Title, + Body: deref(iss.Body), + State: issueState(iss.State), + URL: gqlmodel.URI(b.RepoHTML(owner, repo) + "/issues/" + num), + Locked: iss.Locked, + Closed: iss.State == "closed", + Author: b.gqlActor(iss.User, format), + CreatedAt: gqlmodel.NewDateTime(iss.CreatedAt), + UpdatedAt: gqlmodel.NewDateTime(iss.UpdatedAt), + Labels: b.gqlLabelConnection(owner, repo, iss.Labels, format), + Assignees: b.GQLUserConnection(iss.Assignees, format), + Milestone: b.GQLMilestone(owner, repo, iss.Milestone, format), Comments: &gqlmodel.IssueCommentConnection{TotalCount: int32(iss.CommentsCount), PageInfo: &gqlmodel.PageInfo{}}, ReactionGroups: gqlReactionGroups(iss.Reactions), RepoOwner: owner, diff --git a/presenter/gqlmodel/issue.go b/presenter/gqlmodel/issue.go index ddc07a5..e2aa8ae 100644 --- a/presenter/gqlmodel/issue.go +++ b/presenter/gqlmodel/issue.go @@ -53,7 +53,7 @@ type ReactingUserConnection struct { TotalCount int32 } -// ReactionGroup summarises how many users reacted with a given emoji on an issue. +// ReactionGroup summarizes how many users reacted with a given emoji on an issue. type ReactionGroup struct { Content ReactionContent Users ReactingUserConnection diff --git a/presenter/gqlmodel/repository.go b/presenter/gqlmodel/repository.go index 0b3792f..a6e44e5 100644 --- a/presenter/gqlmodel/repository.go +++ b/presenter/gqlmodel/repository.go @@ -27,47 +27,47 @@ const ( // Nullable GraphQL fields use Go pointers so gqlgen renders null rather than a // zero value. type Repository struct { - ID string // the Repository node ID - Name string // the short repository name - NameWithOwner string // owner login + "/" + name - Description *string // null when unset - IsPrivate bool // visibility - IsFork bool // whether this is a fork - IsArchived bool // whether this is archived - IsEmpty bool // true when the repository has no commits - IsInOrganization bool // true when owner is an org - ForkCount int32 // number of forks - StargazerCount int32 // number of stars - DiskUsage *int32 // disk usage in KB, null when unavailable - HomepageURL *URI // null when unset - CreatedAt DateTime // creation instant - UpdatedAt DateTime // last metadata update - PushedAt *DateTime // last push, null for a repository with no commits - URL URI // the repository's HTML URL - SSHURL GitSSHRemote // the SSH clone URL, GitHub's GitSSHRemote scalar - DatabaseID *int32 // the integer database id REST calls id - Visibility RepositoryVisibility // PUBLIC, PRIVATE, or INTERNAL - ViewerPermission *RepositoryPermission // viewer's permission, null for anonymous - ViewerCanAdminister bool // whether the viewer can administer the repository - ViewerDefaultMergeMethod PullRequestMergeMethod // the viewer's default merge method - HasIssuesEnabled bool // whether the repository accepts issues - HasWikiEnabled bool // whether the repository has a wiki - HasProjectsEnabled bool // whether the repository has projects - HasDiscussionsEnabled bool // whether the repository has discussions (always false) - IsTemplate bool // whether the repository is a template - IsMirror bool // whether the repository is a mirror (always false) - MirrorURL *URI // the mirror source URL (always null) - DeleteBranchOnMerge bool // whether head branches are deleted on merge - AutoMergeAllowed bool // whether auto-merge can be enabled (always true) - MergeCommitAllowed bool // whether merge commits are allowed (always true) - SquashMergeAllowed bool // whether squash merges are allowed (always true) - RebaseMergeAllowed bool // whether rebase merges are allowed (always true) - DefaultBranchRef *Ref // the head branch, null for an empty repository - RepositoryTopics *RepositoryTopicConnection // the repository's topics - Watchers *UserConnection // the users watching the repository - Languages *LanguageConnection // detected languages (always empty) - IssueTemplates []*IssueTemplate // configured issue templates (always null) - PullRequestTemplates []*PullRequestTemplate // configured PR templates (always null) + ID string // the Repository node ID + Name string // the short repository name + NameWithOwner string // owner login + "/" + name + Description *string // null when unset + IsPrivate bool // visibility + IsFork bool // whether this is a fork + IsArchived bool // whether this is archived + IsEmpty bool // true when the repository has no commits + IsInOrganization bool // true when owner is an org + ForkCount int32 // number of forks + StargazerCount int32 // number of stars + DiskUsage *int32 // disk usage in KB, null when unavailable + HomepageURL *URI // null when unset + CreatedAt DateTime // creation instant + UpdatedAt DateTime // last metadata update + PushedAt *DateTime // last push, null for a repository with no commits + URL URI // the repository's HTML URL + SSHURL GitSSHRemote // the SSH clone URL, GitHub's GitSSHRemote scalar + DatabaseID *int32 // the integer database id REST calls id + Visibility RepositoryVisibility // PUBLIC, PRIVATE, or INTERNAL + ViewerPermission *RepositoryPermission // viewer's permission, null for anonymous + ViewerCanAdminister bool // whether the viewer can administer the repository + ViewerDefaultMergeMethod PullRequestMergeMethod // the viewer's default merge method + HasIssuesEnabled bool // whether the repository accepts issues + HasWikiEnabled bool // whether the repository has a wiki + HasProjectsEnabled bool // whether the repository has projects + HasDiscussionsEnabled bool // whether the repository has discussions (always false) + IsTemplate bool // whether the repository is a template + IsMirror bool // whether the repository is a mirror (always false) + MirrorURL *URI // the mirror source URL (always null) + DeleteBranchOnMerge bool // whether head branches are deleted on merge + AutoMergeAllowed bool // whether auto-merge can be enabled (always true) + MergeCommitAllowed bool // whether merge commits are allowed (always true) + SquashMergeAllowed bool // whether squash merges are allowed (always true) + RebaseMergeAllowed bool // whether rebase merges are allowed (always true) + DefaultBranchRef *Ref // the head branch, null for an empty repository + RepositoryTopics *RepositoryTopicConnection // the repository's topics + Watchers *UserConnection // the users watching the repository + Languages *LanguageConnection // detected languages (always empty) + IssueTemplates []*IssueTemplate // configured issue templates (always null) + PullRequestTemplates []*PullRequestTemplate // configured PR templates (always null) // RepoOwner, RepoName, and ForkParentID carry the repository coordinates for // resolvers that look up ref, milestone, release, or parent data on demand. diff --git a/presenter/gqlmodel/user.go b/presenter/gqlmodel/user.go index 932d317..ef8a8fe 100644 --- a/presenter/gqlmodel/user.go +++ b/presenter/gqlmodel/user.go @@ -3,22 +3,22 @@ package gqlmodel // User is the GraphQL User object, carrying the fields gh api and gh auth // status select. It grows toward the full GitHub User type milestone by milestone. type User struct { - ID string // the User node ID - Login string // the user's login - Name *string // display name, null when unset - Email *string // public email, null when unset - Bio *string // profile bio, null when unset - Company *string // profile company, null when unset - Location *string // profile location, null when unset - WebsiteURL *URI // profile blog/website URL, null when unset - TwitterUsername *string // Twitter/X handle without the @, null when unset - DatabaseID *int32 // the integer database id (REST id) - URL URI // the user's profile HTML URL - AvatarURL URI // the user's avatar URL; the size arg appends ?s= - ResourcePath URI // the path part of the profile URL, e.g. /octocat + ID string // the User node ID + Login string // the user's login + Name *string // display name, null when unset + Email *string // public email, null when unset + Bio *string // profile bio, null when unset + Company *string // profile company, null when unset + Location *string // profile location, null when unset + WebsiteURL *URI // profile blog/website URL, null when unset + TwitterUsername *string // Twitter/X handle without the @, null when unset + DatabaseID *int32 // the integer database id (REST id) + URL URI // the user's profile HTML URL + AvatarURL URI // the user's avatar URL; the size arg appends ?s= + ResourcePath URI // the path part of the profile URL, e.g. /octocat Status *UserStatus // the user's set status; always null today - CreatedAt DateTime // account creation instant - UpdatedAt DateTime // last-update instant + CreatedAt DateTime // account creation instant + UpdatedAt DateTime // last-update instant } // IsNode marks User as implementing the Node interface. diff --git a/presenter/gqlpull.go b/presenter/gqlpull.go index ab62b3f..9989ff2 100644 --- a/presenter/gqlpull.go +++ b/presenter/gqlpull.go @@ -202,7 +202,7 @@ func patchStatus(status string) gqlmodel.PatchStatus { // the coordinates the statusCheckRollup resolver reads back. func GQLCommit(repoDBID int64, owner, name string, c git.Commit) *gqlmodel.Commit { return &gqlmodel.Commit{ - ID: nodeid.EncodeGitObject("commit", repoDBID, string(c.SHA)), + ID: nodeid.EncodeGitObject("commit", repoDBID, c.SHA), Oid: gqlmodel.GitObjectID(c.SHA), Message: c.Message, MessageHeadline: messageHeadline(c.Message), diff --git a/presenter/gqlreaction.go b/presenter/gqlreaction.go index 3a28f84..9fa499d 100644 --- a/presenter/gqlreaction.go +++ b/presenter/gqlreaction.go @@ -48,7 +48,7 @@ func GQLReactionContent(rest string) gqlmodel.ReactionContent { return gqlmodel.ReactionContent(rest) } -// GQLReactionGroupsFromList summarises a list of domain reactions into the +// GQLReactionGroupsFromList summarizes a list of domain reactions into the // GraphQL reaction groups, the subject-level rollup addReaction and // removeReaction return after the change. The slice is non-nil even when empty. func GQLReactionGroupsFromList(rs []*domain.Reaction) []*gqlmodel.ReactionGroup { diff --git a/presenter/gqlrepository.go b/presenter/gqlrepository.go index 907a0d2..060159e 100644 --- a/presenter/gqlrepository.go +++ b/presenter/gqlrepository.go @@ -18,44 +18,44 @@ func (b *URLBuilder) GQLRepository(r *domain.Repo, branch *git.Branch, format no perm := gqlmodel.RepositoryPermissionAdmin dbID := int32(r.ID) repo := gqlmodel.Repository{ - ID: nodeid.Encode(nodeid.KindRepository, r.ID, format), - Name: r.Name, - NameWithOwner: r.FullName(), - Description: r.Description, - IsPrivate: r.Private, - IsFork: r.Fork, - IsArchived: r.Archived, - IsEmpty: r.PushedAt == nil, - IsInOrganization: false, // Githome does not yet model organizations - ForkCount: 0, // not stored - StargazerCount: 0, // not stored - HomepageURL: gqlHomepageURL(r.Homepage), - CreatedAt: gqlmodel.NewDateTime(r.CreatedAt), - UpdatedAt: gqlmodel.NewDateTime(r.UpdatedAt), - URL: gqlmodel.URI(b.RepoHTML(r.Owner.Login, r.Name)), - SSHURL: gqlmodel.GitSSHRemote(b.RepoGitSSH(r.Owner.Login, r.Name)), - DatabaseID: &dbID, - Visibility: gqlVisibility(r.Private), - ViewerPermission: &perm, // all authenticated users get ADMIN on their own repos - ViewerCanAdminister: true, + ID: nodeid.Encode(nodeid.KindRepository, r.ID, format), + Name: r.Name, + NameWithOwner: r.FullName(), + Description: r.Description, + IsPrivate: r.Private, + IsFork: r.Fork, + IsArchived: r.Archived, + IsEmpty: r.PushedAt == nil, + IsInOrganization: false, // Githome does not yet model organizations + ForkCount: 0, // not stored + StargazerCount: 0, // not stored + HomepageURL: gqlHomepageURL(r.Homepage), + CreatedAt: gqlmodel.NewDateTime(r.CreatedAt), + UpdatedAt: gqlmodel.NewDateTime(r.UpdatedAt), + URL: gqlmodel.URI(b.RepoHTML(r.Owner.Login, r.Name)), + SSHURL: gqlmodel.GitSSHRemote(b.RepoGitSSH(r.Owner.Login, r.Name)), + DatabaseID: &dbID, + Visibility: gqlVisibility(r.Private), + ViewerPermission: &perm, // all authenticated users get ADMIN on their own repos + ViewerCanAdminister: true, ViewerDefaultMergeMethod: gqlmodel.PullRequestMergeMethodMerge, - HasIssuesEnabled: r.HasIssues, - HasWikiEnabled: r.HasWiki, - HasProjectsEnabled: r.HasProjects, - HasDiscussionsEnabled: false, // discussions are not modeled - IsTemplate: r.IsTemplate, - IsMirror: false, // Githome does not mirror - DeleteBranchOnMerge: r.DeleteBranchOnMerge, - AutoMergeAllowed: true, - MergeCommitAllowed: true, - SquashMergeAllowed: true, - RebaseMergeAllowed: true, - RepositoryTopics: b.gqlRepositoryTopics(r), - Watchers: &gqlmodel.UserConnection{Nodes: []*gqlmodel.User{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0}, - Languages: &gqlmodel.LanguageConnection{Nodes: []*gqlmodel.Language{}, Edges: []*gqlmodel.LanguageEdge{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0, TotalSize: 0}, - RepoOwner: r.Owner.Login, - RepoName: r.Name, - ForkParentPK: r.ForkOfPK, + HasIssuesEnabled: r.HasIssues, + HasWikiEnabled: r.HasWiki, + HasProjectsEnabled: r.HasProjects, + HasDiscussionsEnabled: false, // discussions are not modeled + IsTemplate: r.IsTemplate, + IsMirror: false, // Githome does not mirror + DeleteBranchOnMerge: r.DeleteBranchOnMerge, + AutoMergeAllowed: true, + MergeCommitAllowed: true, + SquashMergeAllowed: true, + RebaseMergeAllowed: true, + RepositoryTopics: b.gqlRepositoryTopics(r), + Watchers: &gqlmodel.UserConnection{Nodes: []*gqlmodel.User{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0}, + Languages: &gqlmodel.LanguageConnection{Nodes: []*gqlmodel.Language{}, Edges: []*gqlmodel.LanguageEdge{}, PageInfo: &gqlmodel.PageInfo{}, TotalCount: 0, TotalSize: 0}, + RepoOwner: r.Owner.Login, + RepoName: r.Name, + ForkParentPK: r.ForkOfPK, } if r.PushedAt != nil { pushed := gqlmodel.NewDateTime(*r.PushedAt) diff --git a/presenter/notification.go b/presenter/notification.go index 13b4b52..1c35d6a 100644 --- a/presenter/notification.go +++ b/presenter/notification.go @@ -37,7 +37,6 @@ func (b *URLBuilder) NotificationThread(t *domain.NotificationThread, repo *doma } } -// NotificationSubscription renders the thread's subscription state. // RepoSubscription renders a repository subscription. The url is the // subscription endpoint and repository_url the repository itself, the pair // GitHub returns on this body. @@ -52,6 +51,7 @@ func (b *URLBuilder) RepoSubscription(s *domain.Subscription) restmodel.RepoSubs } } +// NotificationSubscription renders the thread's subscription state. func (b *URLBuilder) NotificationSubscription(t *domain.NotificationThread) restmodel.NotificationSubscription { id := strconv.FormatInt(t.ID, 10) return restmodel.NotificationSubscription{ diff --git a/presenter/restmodel/gist.go b/presenter/restmodel/gist.go index c08fd1e..6af7674 100644 --- a/presenter/restmodel/gist.go +++ b/presenter/restmodel/gist.go @@ -13,25 +13,25 @@ type GistFile struct { // Gist is the body returned by every gist endpoint. type Gist struct { - ID string `json:"id"` - NodeID string `json:"node_id"` - Description string `json:"description"` - Public bool `json:"public"` - Owner SimpleUser `json:"owner"` - User *SimpleUser `json:"user"` - Files map[string]GistFile `json:"files"` - Forks []any `json:"forks"` - History []any `json:"history"` - Truncated bool `json:"truncated"` - Comments int `json:"comments"` - GitPullURL string `json:"git_pull_url"` - GitPushURL string `json:"git_push_url"` - HTMLURL string `json:"html_url"` - CommitsURL string `json:"commits_url"` - ForksURL string `json:"forks_url"` - CommentsURL string `json:"comments_url"` - CreatedAt Time `json:"created_at"` - UpdatedAt Time `json:"updated_at"` + ID string `json:"id"` + NodeID string `json:"node_id"` + Description string `json:"description"` + Public bool `json:"public"` + Owner SimpleUser `json:"owner"` + User *SimpleUser `json:"user"` + Files map[string]GistFile `json:"files"` + Forks []any `json:"forks"` + History []any `json:"history"` + Truncated bool `json:"truncated"` + Comments int `json:"comments"` + GitPullURL string `json:"git_pull_url"` + GitPushURL string `json:"git_push_url"` + HTMLURL string `json:"html_url"` + CommitsURL string `json:"commits_url"` + ForksURL string `json:"forks_url"` + CommentsURL string `json:"comments_url"` + CreatedAt Time `json:"created_at"` + UpdatedAt Time `json:"updated_at"` } // GistComment is the body returned by gist comment endpoints. diff --git a/presenter/restmodel/search.go b/presenter/restmodel/search.go index 9e9f8bc..15ca1b3 100644 --- a/presenter/restmodel/search.go +++ b/presenter/restmodel/search.go @@ -65,12 +65,12 @@ type SearchCode struct { // the blob object id, the API and HTML URLs that address it, the repository it // lives in, and the score. type CodeSearchItem struct { - Name string `json:"name"` - Path string `json:"path"` - SHA string `json:"sha"` - URL string `json:"url"` - GitURL string `json:"git_url"` - HTMLURL string `json:"html_url"` + Name string `json:"name"` + Path string `json:"path"` + SHA string `json:"sha"` + URL string `json:"url"` + GitURL string `json:"git_url"` + HTMLURL string `json:"html_url"` Repository Repository `json:"repository"` Score float64 `json:"score"` TextMatches []TextMatch `json:"text_matches,omitempty"` diff --git a/store/queries_github_app.go b/store/queries_github_app.go index d85c6b4..b843446 100644 --- a/store/queries_github_app.go +++ b/store/queries_github_app.go @@ -112,7 +112,7 @@ func (s *Store) InstallationsByAppPK(ctx context.Context, appPK int64) ([]*Insta if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*InstallationRow for rows.Next() { r, err := scanInstallation(rows) @@ -140,7 +140,7 @@ func (s *Store) InstallationRepoPKs(ctx context.Context, instPK int64) ([]int64, if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []int64 for rows.Next() { var pk int64 @@ -173,9 +173,9 @@ func scanGitHubApp(row interface{ Scan(...any) error }) (*GitHubAppRow, error) { func scanInstallation(row interface{ Scan(...any) error }) (*InstallationRow, error) { var ( - r InstallationRow - suspended nullTime - created nullTime + r InstallationRow + suspended nullTime + created nullTime ) err := row.Scan( &r.PK, &r.DBID, &r.AppPK, &r.AccountPK, &r.RepositorySelection, diff --git a/store/queries_keys.go b/store/queries_keys.go index 6d19a58..6d131e1 100644 --- a/store/queries_keys.go +++ b/store/queries_keys.go @@ -156,7 +156,7 @@ func scanSSHKeys(rows *sql.Rows, err error) ([]*SSHKeyRow, error) { if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*SSHKeyRow for rows.Next() { k, err := scanSSHKeyRow(rows) diff --git a/store/queries_notifications.go b/store/queries_notifications.go index 8edfb9f..7889d3e 100644 --- a/store/queries_notifications.go +++ b/store/queries_notifications.go @@ -76,7 +76,7 @@ func (s *Store) ListNotificationThreads(ctx context.Context, userPK, repoPK int6 if err != nil { return nil, 0, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*NotificationThreadRow for rows.Next() { t, err := scanNotificationThreadRow(rows) diff --git a/store/queries_repo.go b/store/queries_repo.go index 57af3c6..37121b4 100644 --- a/store/queries_repo.go +++ b/store/queries_repo.go @@ -220,7 +220,7 @@ func (s *Store) ReposByOwner(ctx context.Context, ownerPK int64) ([]*RepoRow, er if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -247,7 +247,7 @@ func (s *Store) ListPublicRepos(ctx context.Context, sinceDBID int64, limit int) if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -294,7 +294,7 @@ func (s *Store) ForksOf(ctx context.Context, pk int64) ([]*RepoRow, error) { if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -319,7 +319,7 @@ func (s *Store) ReposByCollaborator(ctx context.Context, userPK int64) ([]*RepoR if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) @@ -345,7 +345,7 @@ func (s *Store) ReposByTeamMember(ctx context.Context, userPK int64) ([]*RepoRow if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []*RepoRow for rows.Next() { r, err := scanRepo(rows) diff --git a/store/store_test.go b/store/store_test.go index 5d67a8e..25fe3ee 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -189,7 +189,7 @@ func TestReadsRunConcurrentWithWriteTx(t *testing.T) { release := make(chan struct{}) txDone := make(chan error, 1) go func() { - txDone <- st.WithTx(ctx, func(tx *store.Tx) error { + txDone <- st.WithTx(ctx, func(_ *store.Tx) error { close(txStarted) <-release return nil From e2cb9234e8b3c663a20366d26618b4a9307eb91d Mon Sep 17 00:00:00 2001 From: Duc-Tam Nguyen Date: Sat, 13 Jun 2026 19:15:11 +0700 Subject: [PATCH 2/5] store: fix Postgres rollback of the github_apps migration The 0012 down dropped the installations and github_apps tables before dropping the tokens columns whose foreign keys point at them, so Postgres refused with "cannot drop table ... because other objects depend on it" and the rollback stopped halfway. SQLite does not enforce the dependency on drop, so only the Postgres path was affected. Drop the token linkage columns first, then the tables in reverse dependency order. This is what left every store test's database dirty between runs on the Postgres leg: the per-test reset rolls all the way down and back up, and a rollback that bailed early kept old rows around, which then collided on unique logins. --- store/migrations/0012_github_apps.pg.down.sql | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/store/migrations/0012_github_apps.pg.down.sql b/store/migrations/0012_github_apps.pg.down.sql index 600721f..60e6154 100644 --- a/store/migrations/0012_github_apps.pg.down.sql +++ b/store/migrations/0012_github_apps.pg.down.sql @@ -1,7 +1,9 @@ --- 0012_github_apps (postgres, down) -DROP TABLE IF EXISTS installation_repositories; -DROP TABLE IF EXISTS installations; -DROP TABLE IF EXISTS github_apps; +-- 0012_github_apps (postgres, down): drop the token linkage columns first so the +-- foreign keys they carry stop pinning installations and github_apps, then drop +-- the tables in reverse dependency order. ALTER TABLE tokens DROP COLUMN IF EXISTS installation_pk; ALTER TABLE tokens DROP COLUMN IF EXISTS github_app_pk; ALTER TABLE tokens DROP COLUMN IF EXISTS grant_json; +DROP TABLE IF EXISTS installation_repositories; +DROP TABLE IF EXISTS installations; +DROP TABLE IF EXISTS github_apps; From 2e1d9012e2db5432b28fd2d59c245522144b3a1a Mon Sep 17 00:00:00 2001 From: Duc-Tam Nguyen Date: Sat, 13 Jun 2026 19:15:19 +0700 Subject: [PATCH 3/5] store: fold missing migrations into the Postgres consolidated schema schema.pg.sql had drifted badly: it stopped tracking migrations after 0011 and was missing the tables added by 0012 through 0021 (github apps, keys and branch protections, teams and collaborators, oauth auth codes, gists, notifications, code search, and the list indexes). Install applies this file in one shot, so a fresh Postgres install was broken and the 0032 ALTERs even referenced a branch_protections table that was never created. Fold the missing migrations back in, in version order. Add TestPostgresSchemaFileMatchesMigrations as the Postgres counterpart to the SQLite drift guard. It builds one database with Migrate and another with Install and compares their structure column by column and index by index. The SQLite half had this guard; the Postgres half never did, which is how the file drifted unnoticed. --- store/migrate_install_test.go | 66 ++++++++++ store/migrations/schema.pg.sql | 223 +++++++++++++++++++++++++++++++++ 2 files changed, 289 insertions(+) diff --git a/store/migrate_install_test.go b/store/migrate_install_test.go index cf66f8e..9c8c1db 100644 --- a/store/migrate_install_test.go +++ b/store/migrate_install_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "os" "path/filepath" "sort" "strings" @@ -91,6 +92,71 @@ func TestInstallStampsLedger(t *testing.T) { }) } +// TestPostgresSchemaFileMatchesMigrations is the drift guard for schema.pg.sql, +// the Postgres counterpart to TestSchemaFileMatchesMigrations. A database built +// by Install (the consolidated schema file) must have the same structure as one +// built by replaying every migration. It runs only where a Postgres DSN is set. +// schema.pg.sql is maintained by hand, so this catches a migration that was +// added without folding its tables and columns into the consolidated file. +func TestPostgresSchemaFileMatchesMigrations(t *testing.T) { + dsn := os.Getenv("GITHOME_TEST_POSTGRES_DSN") + if dsn == "" { + t.Skip("set GITHOME_TEST_POSTGRES_DSN to run the Postgres schema drift guard") + } + ctx := context.Background() + + fingerprint := func(build func(*store.Store) error) string { + t.Helper() + st, err := store.Open(ctx, dsn) + if err != nil { + t.Fatalf("open: %v", err) + } + defer func() { _ = st.Close() }() + // Reach an empty schema before building, so a reused database does not + // taint the comparison. + if err := st.Rollback(ctx, 1<<20); err != nil { + t.Fatalf("reset: %v", err) + } + if err := build(st); err != nil { + t.Fatalf("build: %v", err) + } + return pgFingerprint(t, st.DB()) + } + + want := fingerprint(func(st *store.Store) error { return st.Migrate(ctx) }) + got := fingerprint(func(st *store.Store) error { return st.Install(ctx) }) + if want != got { + t.Fatalf("schema.pg.sql drifted from the migrations.\n--- migrated ---\n%s\n--- installed ---\n%s", want, got) + } +} + +// pgFingerprint renders a stable description of a Postgres database's structure: +// every application table's columns (type, nullability, default, and any +// generated expression) and every index, keyed and ordered by name so the +// column order Install and Migrate happen to write in does not matter. The +// schema_migrations ledger is excluded so the two build paths are compared on +// application schema alone. +func pgFingerprint(t *testing.T, db *sql.DB) string { + t.Helper() + var b strings.Builder + cols := queryStrings(t, db, `SELECT table_name||'.'||column_name||' '||data_type + ||' null='||is_nullable + ||' dflt='||coalesce(column_default,'') + ||' gen='||coalesce(generation_expression,'') + FROM information_schema.columns + WHERE table_schema='public' AND table_name <> 'schema_migrations' + ORDER BY 1`) + for _, c := range cols { + fmt.Fprintf(&b, "COLUMN %s\n", c) + } + idx := queryStrings(t, db, `SELECT indexname||' :: '||indexdef FROM pg_indexes + WHERE schemaname='public' AND tablename <> 'schema_migrations' ORDER BY 1`) + for _, i := range idx { + fmt.Fprintf(&b, "INDEX %s\n", i) + } + return b.String() +} + // sqliteFingerprint renders a stable, whitespace-normalized description of a // SQLite database's structure: every user table's columns, every named index, // and every trigger. The schema_migrations ledger and SQLite's internal objects diff --git a/store/migrations/schema.pg.sql b/store/migrations/schema.pg.sql index b655a92..9950d8c 100644 --- a/store/migrations/schema.pg.sql +++ b/store/migrations/schema.pg.sql @@ -483,6 +483,229 @@ CREATE TABLE release_assets ( ); CREATE UNIQUE INDEX release_assets_release_name_uq ON release_assets (release_pk, name) WHERE deleted_at IS NULL; +-- 0012_github_apps +CREATE TABLE github_apps ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE DEFAULT nextval('global_id_seq'), + owner_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + slug TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + client_id TEXT NOT NULL UNIQUE, + client_secret_hash BYTEA, + webhook_secret_hash BYTEA, + private_key_pem BYTEA NOT NULL, + permissions JSONB NOT NULL DEFAULT '{}', + events TEXT[] NOT NULL DEFAULT '{}', + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE TABLE installations ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE DEFAULT nextval('global_id_seq'), + app_pk BIGINT NOT NULL REFERENCES github_apps(pk) ON DELETE CASCADE, + account_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + repository_selection TEXT NOT NULL DEFAULT 'all', + permissions JSONB NOT NULL DEFAULT '{}', + events TEXT[] NOT NULL DEFAULT '{}', + suspended_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX installations_app_account_uq ON installations (app_pk, account_pk); + +CREATE TABLE installation_repositories ( + installation_pk BIGINT NOT NULL REFERENCES installations(pk) ON DELETE CASCADE, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + PRIMARY KEY (installation_pk, repo_pk) +); + +ALTER TABLE tokens ADD COLUMN installation_pk BIGINT REFERENCES installations(pk) ON DELETE CASCADE; +ALTER TABLE tokens ADD COLUMN github_app_pk BIGINT REFERENCES github_apps(pk) ON DELETE CASCADE; +ALTER TABLE tokens ADD COLUMN grant_json TEXT; + +-- 0013_keys_protection +CREATE TABLE ssh_keys ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE DEFAULT nextval('global_id_seq'), + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + title TEXT, + key_type TEXT NOT NULL, + public_key TEXT NOT NULL, + fingerprint TEXT NOT NULL, + read_only BOOLEAN NOT NULL DEFAULT FALSE, + repo_pk BIGINT REFERENCES repositories(pk) ON DELETE CASCADE, + last_used_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX ssh_keys_fp_uq ON ssh_keys (fingerprint); + +CREATE TABLE branch_protections ( + pk BIGSERIAL PRIMARY KEY, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + branch_pattern TEXT NOT NULL, + require_pr_reviews BOOLEAN NOT NULL DEFAULT FALSE, + required_approving_count INT NOT NULL DEFAULT 0, + dismiss_stale_reviews BOOLEAN NOT NULL DEFAULT FALSE, + require_code_owner_reviews BOOLEAN NOT NULL DEFAULT FALSE, + require_status_checks BOOLEAN NOT NULL DEFAULT FALSE, + require_branches_up_to_date BOOLEAN NOT NULL DEFAULT FALSE, + status_check_contexts TEXT[] NOT NULL DEFAULT '{}', + enforce_admins BOOLEAN NOT NULL DEFAULT FALSE, + restrictions_users TEXT[] NOT NULL DEFAULT '{}', + restrictions_teams TEXT[] NOT NULL DEFAULT '{}', + allow_force_pushes BOOLEAN NOT NULL DEFAULT FALSE, + allow_deletions BOOLEAN NOT NULL DEFAULT FALSE, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX branch_protections_repo_pattern_uq ON branch_protections (repo_pk, branch_pattern); + +-- 0014_teams_collab_topics +ALTER TABLE repositories ADD COLUMN IF NOT EXISTS topics TEXT NOT NULL DEFAULT '[]'; + +CREATE TABLE IF NOT EXISTS teams ( + pk BIGSERIAL PRIMARY KEY, + db_id BIGINT NOT NULL UNIQUE, + org_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + name TEXT NOT NULL, + slug TEXT NOT NULL, + description TEXT, + privacy TEXT NOT NULL DEFAULT 'secret', + permission TEXT NOT NULL DEFAULT 'pull', + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX IF NOT EXISTS teams_org_slug_uq ON teams (org_pk, slug); + +CREATE TABLE IF NOT EXISTS team_members ( + pk BIGSERIAL PRIMARY KEY, + team_pk BIGINT NOT NULL REFERENCES teams(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + role TEXT NOT NULL DEFAULT 'member', + UNIQUE (team_pk, user_pk) +); + +CREATE TABLE IF NOT EXISTS team_repos ( + pk BIGSERIAL PRIMARY KEY, + team_pk BIGINT NOT NULL REFERENCES teams(pk) ON DELETE CASCADE, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + permission TEXT NOT NULL DEFAULT 'pull', + UNIQUE (team_pk, repo_pk) +); + +CREATE TABLE IF NOT EXISTS collaborators ( + pk BIGSERIAL PRIMARY KEY, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + permission TEXT NOT NULL DEFAULT 'push', + UNIQUE (repo_pk, user_pk) +); + +-- 0015_oauth_auth_codes +CREATE TABLE IF NOT EXISTS oauth_auth_codes ( + pk BIGSERIAL PRIMARY KEY, + code_hash BYTEA NOT NULL UNIQUE, + oauth_app_pk BIGINT NOT NULL REFERENCES oauth_apps(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + redirect_uri TEXT NOT NULL, + scopes TEXT NOT NULL DEFAULT '', + used BOOLEAN NOT NULL DEFAULT false, + expires_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- 0016_gists +CREATE TABLE gists ( + pk BIGSERIAL PRIMARY KEY, + gist_id TEXT NOT NULL UNIQUE, + owner_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + description TEXT NOT NULL DEFAULT '', + public BOOLEAN NOT NULL DEFAULT TRUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX gists_owner ON gists (owner_pk); + +CREATE TABLE gist_files ( + pk BIGSERIAL PRIMARY KEY, + gist_pk BIGINT NOT NULL REFERENCES gists(pk) ON DELETE CASCADE, + filename TEXT NOT NULL, + content TEXT NOT NULL DEFAULT '', + UNIQUE(gist_pk, filename) +); + +CREATE TABLE gist_stars ( + gist_pk BIGINT NOT NULL REFERENCES gists(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + PRIMARY KEY(gist_pk, user_pk) +); + +CREATE TABLE gist_comments ( + pk BIGSERIAL PRIMARY KEY, + gist_pk BIGINT NOT NULL REFERENCES gists(pk) ON DELETE CASCADE, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + body TEXT NOT NULL DEFAULT '', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- 0017_oauth_app_callback +ALTER TABLE oauth_apps ADD COLUMN IF NOT EXISTS callback_url TEXT NOT NULL DEFAULT ''; + +-- 0018_protection_restrictions +ALTER TABLE branch_protections ADD COLUMN IF NOT EXISTS restrictions_enabled BOOLEAN NOT NULL DEFAULT FALSE; + +-- 0019_notifications +CREATE TABLE IF NOT EXISTS notification_threads ( + pk BIGSERIAL PRIMARY KEY, + user_pk BIGINT NOT NULL REFERENCES users(pk) ON DELETE CASCADE, + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + issue_pk BIGINT NOT NULL REFERENCES issues(pk) ON DELETE CASCADE, + reason TEXT NOT NULL DEFAULT 'subscribed', + unread BOOLEAN NOT NULL DEFAULT TRUE, + subscribed BOOLEAN NOT NULL DEFAULT TRUE, + ignored BOOLEAN NOT NULL DEFAULT FALSE, + last_read_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX IF NOT EXISTS notification_threads_user_issue_uq ON notification_threads (user_pk, issue_pk); +CREATE INDEX IF NOT EXISTS notification_threads_user_updated ON notification_threads (user_pk, updated_at); + +-- 0020_code_search +CREATE TABLE code_documents ( + repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, + path TEXT NOT NULL, + sha TEXT NOT NULL, + content TEXT NOT NULL DEFAULT '', + search_vector tsvector GENERATED ALWAYS AS ( + to_tsvector('simple', regexp_replace(path, '[^A-Za-z0-9]+', ' ', 'g') || ' ' || content) + ) STORED, + PRIMARY KEY (repo_pk, path) +); +CREATE INDEX code_documents_search_vector_gin ON code_documents USING GIN (search_vector); + +CREATE TABLE code_index_state ( + repo_pk BIGINT PRIMARY KEY REFERENCES repositories(pk) ON DELETE CASCADE, + head_sha TEXT NOT NULL, + truncated BOOLEAN NOT NULL DEFAULT FALSE, + indexed_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- 0021_list_indexes +CREATE INDEX issues_repo_updated_number_idx + ON issues (repo_pk, updated_at DESC, number DESC) + WHERE deleted_at IS NULL; +CREATE INDEX issues_repo_comments_number_idx + ON issues (repo_pk, comments_count DESC, number DESC) + WHERE deleted_at IS NULL; +CREATE INDEX issue_labels_label_idx ON issue_labels (label_pk); +CREATE INDEX gists_public_updated_idx ON gists (public, updated_at DESC); +DROP INDEX issue_comments_issue_idx; +CREATE INDEX issue_comments_issue_created_pk_idx + ON issue_comments (issue_pk, created_at, pk) + WHERE deleted_at IS NULL; + + -- 0022_repo_redirects CREATE TABLE repo_redirects ( pk BIGSERIAL PRIMARY KEY, From 841bbe1273a71b87d386c7d94ad2f94570267788 Mon Sep 17 00:00:00 2001 From: Duc-Tam Nguyen Date: Sat, 13 Jun 2026 19:15:26 +0700 Subject: [PATCH 4/5] store: make Postgres code search match paths and order like SQLite Two Postgres-only code search bugs surfaced once the leg ran clean. A search for one path segment, say "logo", did not find assets/logo.png. Postgres's text parser keeps a slash-joined path as a single file token, where SQLite's FTS5 tokenizer splits on every non-word character. Split the path on its punctuation before building the tsvector so both dialects index the same terms. The no-terms listing came back in a different order. The query ordered by path, which SQLite sorts bytewise but Postgres sorts by the cluster collation, so mixed-case paths interleaved differently. Pin the Postgres sort to the C collation so both dialects page in the same order. --- store/migrations/0020_code_search.pg.up.sql | 8 +++++++- store/queries_code.go | 13 ++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/store/migrations/0020_code_search.pg.up.sql b/store/migrations/0020_code_search.pg.up.sql index b2c0ec9..e93b322 100644 --- a/store/migrations/0020_code_search.pg.up.sql +++ b/store/migrations/0020_code_search.pg.up.sql @@ -3,6 +3,12 @@ -- dictionary does no stemming or stop-word removal, matching FTS5's tokenizer -- closely enough that both dialects answer the same queries. The indexer -- replaces a repository's rows wholesale on reindex. +-- +-- The path is split on its punctuation before tokenizing so a query for one +-- segment ("logo") finds "assets/logo.png". Postgres's text parser otherwise +-- keeps a slash-joined path as a single "file" token, which FTS5's unicode61 +-- tokenizer would have split on every non-word character. Replacing those +-- characters with spaces lines the two dialects up. CREATE TABLE code_documents ( repo_pk BIGINT NOT NULL REFERENCES repositories(pk) ON DELETE CASCADE, @@ -10,7 +16,7 @@ CREATE TABLE code_documents ( sha TEXT NOT NULL, content TEXT NOT NULL DEFAULT '', search_vector tsvector GENERATED ALWAYS AS ( - to_tsvector('simple', path || ' ' || content) + to_tsvector('simple', regexp_replace(path, '[^A-Za-z0-9]+', ' ', 'g') || ' ' || content) ) STORED, PRIMARY KEY (repo_pk, path) ); diff --git a/store/queries_code.go b/store/queries_code.go index 3fd13d9..885aadb 100644 --- a/store/queries_code.go +++ b/store/queries_code.go @@ -62,6 +62,17 @@ func (s *Store) codeTable() string { return "code_fts" } +// codePathOrder returns the ORDER BY expression for path. SQLite sorts text +// bytewise (BINARY); Postgres sorts by the cluster's collation, which orders +// mixed-case paths differently. Pinning the Postgres sort to the "C" collation +// makes both dialects page in the same byte order. +func (s *Store) codePathOrder() string { + if s.dialect == DialectPostgres { + return `path COLLATE "C"` + } + return "path" +} + // SearchCode returns the page of indexed files matching the query, ordered by // repository then path so pagination is deterministic. func (s *Store) SearchCode(ctx context.Context, q CodeSearch) ([]CodeHit, error) { @@ -78,7 +89,7 @@ func (s *Store) SearchCode(ctx context.Context, q CodeSearch) ([]CodeHit, error) } args = append(args, limit, q.Offset) sql := s.rebind(`SELECT repo_pk, path, sha FROM ` + s.codeTable() + where + - ` ORDER BY repo_pk, path LIMIT ? OFFSET ?`) + ` ORDER BY repo_pk, ` + s.codePathOrder() + ` LIMIT ? OFFSET ?`) rows, err := s.rdb.QueryContext(ctx, sql, args...) if err != nil { return nil, err From fa8c4fedbb92a41fd9be701bd80d55efecdb2790 Mon Sep 17 00:00:00 2001 From: Duc-Tam Nguyen Date: Sat, 13 Jun 2026 19:34:22 +0700 Subject: [PATCH 5/5] graphql: stop the loader batch tests racing the 1ms window The loader batch tests fan out N goroutines and assert they collapse into a single fetch. They reused the production 1ms coalescing window, so a loaded race-mode runner that took longer than 1ms to schedule all N goroutines split the wave into two fetches and failed the count. This is what reddened the sqlite CI leg on TestLoaderLabelsAndAssigneesBatch. Thread the batch window through newLoaders instead of hardcoding it. Production still passes loaderWait; the tests pass a generous 250ms so goroutine scheduling can never break up a logical wave. The coalescing behavior under test is unchanged, only the deadline is no longer a race. --- api/graphql/loaders.go | 16 +++++++++------- api/graphql/loaders_test.go | 7 ++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/api/graphql/loaders.go b/api/graphql/loaders.go index 2a9a658..2a3afbf 100644 --- a/api/graphql/loaders.go +++ b/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)) }) } diff --git a/api/graphql/loaders_test.go b/api/graphql/loaders_test.go index a6c63f2..cd00e62 100644 --- a/api/graphql/loaders_test.go +++ b/api/graphql/loaders_test.go @@ -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