tenseleyflow/shithub / 52ab4e1

Browse files

H2/H3: sync default_branch_oid post-merge; delete dead KindPRMerge

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
52ab4e1f23e76e4abfdbce7b80edf0bcdad9b2b7
Parents
89bc712
Tree
2661f2d

4 changed files

StatusFile+-
M cmd/shithubd/worker.go 0 1
M internal/pulls/merge.go 32 11
M internal/worker/jobs/pr_jobs.go 0 35
M internal/worker/types.go 7 4
cmd/shithubd/worker.gomodified
@@ -95,7 +95,6 @@ var workerCmd = &cobra.Command{
95
 		prDeps := jobs.PRJobsDeps{Pool: pool, RepoFS: rfs, Logger: logger}
95
 		prDeps := jobs.PRJobsDeps{Pool: pool, RepoFS: rfs, Logger: logger}
96
 		p.Register(worker.KindPRSynchronize, jobs.PRSynchronize(prDeps))
96
 		p.Register(worker.KindPRSynchronize, jobs.PRSynchronize(prDeps))
97
 		p.Register(worker.KindPRMergeability, jobs.PRMergeability(prDeps))
97
 		p.Register(worker.KindPRMergeability, jobs.PRMergeability(prDeps))
98
-		p.Register(worker.KindPRMerge, jobs.PRMerge(prDeps))
99
 
98
 
100
 		return p.Run(ctx)
99
 		return p.Run(ctx)
101
 	},
100
 	},
internal/pulls/merge.gomodified
@@ -4,7 +4,6 @@ package pulls
4
 
4
 
5
 import (
5
 import (
6
 	"context"
6
 	"context"
7
-	"errors"
8
 	"fmt"
7
 	"fmt"
9
 	"strconv"
8
 	"strconv"
10
 	"strings"
9
 	"strings"
@@ -12,12 +11,13 @@ import (
12
 
11
 
13
 	"github.com/jackc/pgx/v5/pgtype"
12
 	"github.com/jackc/pgx/v5/pgtype"
14
 
13
 
14
+	"github.com/tenseleyFlow/shithub/internal/auth/audit"
15
 	"github.com/tenseleyFlow/shithub/internal/checks"
15
 	"github.com/tenseleyFlow/shithub/internal/checks"
16
-	"github.com/tenseleyFlow/shithub/internal/issues"
17
 	issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc"
16
 	issuesdb "github.com/tenseleyFlow/shithub/internal/issues/sqlc"
18
 	"github.com/tenseleyFlow/shithub/internal/pulls/review"
17
 	"github.com/tenseleyFlow/shithub/internal/pulls/review"
19
 	pullsdb "github.com/tenseleyFlow/shithub/internal/pulls/sqlc"
18
 	pullsdb "github.com/tenseleyFlow/shithub/internal/pulls/sqlc"
20
 	repogit "github.com/tenseleyFlow/shithub/internal/repos/git"
19
 	repogit "github.com/tenseleyFlow/shithub/internal/repos/git"
20
+	reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc"
21
 	usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc"
21
 	usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc"
22
 )
22
 )
23
 
23
 
@@ -163,16 +163,35 @@ func Merge(ctx context.Context, deps Deps, p MergeParams) error {
163
 	}
163
 	}
164
 
164
 
165
 	if err := q.SetPullRequestMerged(ctx, tx, pullsdb.SetPullRequestMergedParams{
165
 	if err := q.SetPullRequestMerged(ctx, tx, pullsdb.SetPullRequestMergedParams{
166
-		IssueID:         p.PRID,
166
+		IssueID:        p.PRID,
167
-		MergedByUserID:  pgtype.Int8{Int64: p.ActorUserID, Valid: p.ActorUserID != 0},
167
+		MergedByUserID: pgtype.Int8{Int64: p.ActorUserID, Valid: p.ActorUserID != 0},
168
-		MergeCommitSha:  pgtype.Text{String: mergeRes.MergedOID, Valid: true},
168
+		MergeCommitSha: pgtype.Text{String: mergeRes.MergedOID, Valid: true},
169
-		MergeMethod:     pullsdb.NullPrMergeMethod{PrMergeMethod: pullsdb.PrMergeMethod(p.Method), Valid: true},
169
+		MergeMethod:    pullsdb.NullPrMergeMethod{PrMergeMethod: pullsdb.PrMergeMethod(p.Method), Valid: true},
170
-		BaseOidAtMerge:  pgtype.Text{String: pr.BaseOid, Valid: true},
170
+		BaseOidAtMerge: pgtype.Text{String: pr.BaseOid, Valid: true},
171
-		HeadOidAtMerge:  pgtype.Text{String: pr.HeadOid, Valid: true},
171
+		HeadOidAtMerge: pgtype.Text{String: pr.HeadOid, Valid: true},
172
 	}); err != nil {
172
 	}); err != nil {
173
 		return err
173
 		return err
174
 	}
174
 	}
175
 
175
 
176
+	// PerformMerge moved refs/heads/<base> on disk via update-ref,
177
+	// bypassing the post-receive hook that would normally maintain
178
+	// repos.default_branch_oid. When the merged base IS the repo's
179
+	// default branch, sync the column ourselves so the repo home view
180
+	// stays accurate. The audit caught this gap (S00-S25, H2).
181
+	rq := reposdb.New()
182
+	repo, err := rq.GetRepoByID(ctx, tx, issue.RepoID)
183
+	if err != nil {
184
+		return fmt.Errorf("merge: load repo: %w", err)
185
+	}
186
+	if repo.DefaultBranch == pr.BaseRef {
187
+		if err := rq.UpdateRepoDefaultBranchOID(ctx, tx, reposdb.UpdateRepoDefaultBranchOIDParams{
188
+			ID:               repo.ID,
189
+			DefaultBranchOid: pgtype.Text{String: mergeRes.MergedOID, Valid: true},
190
+		}); err != nil {
191
+			return fmt.Errorf("merge: update default_branch_oid: %w", err)
192
+		}
193
+	}
194
+
176
 	// Close the issue side with state_reason=completed.
195
 	// Close the issue side with state_reason=completed.
177
 	if err := iq.SetIssueState(ctx, tx, issuesdb.SetIssueStateParams{
196
 	if err := iq.SetIssueState(ctx, tx, issuesdb.SetIssueStateParams{
178
 		ID:             p.PRID,
197
 		ID:             p.PRID,
@@ -233,6 +252,11 @@ func Merge(ctx context.Context, deps Deps, p MergeParams) error {
233
 		return err
252
 		return err
234
 	}
253
 	}
235
 	committed = true
254
 	committed = true
255
+	if deps.Audit != nil {
256
+		_ = deps.Audit.Record(ctx, deps.Pool, p.ActorUserID,
257
+			audit.ActionPullMerged, audit.TargetPull, p.PRID,
258
+			map[string]any{"method": p.Method, "commit": mergeRes.MergedOID})
259
+	}
236
 	return nil
260
 	return nil
237
 }
261
 }
238
 
262
 
@@ -273,6 +297,3 @@ func fetchCommitsForLinkScan(ctx context.Context, db pullsdb.DBTX, prID int64) [
273
 	return out
297
 	return out
274
 }
298
 }
275
 
299
 
276
-// Compile-time check that issues' typed errors are still in scope so
277
-// EditPR's wrapping continues to work after refactors.
278
-var _ = errors.Is(issues.ErrEmptyTitle, issues.ErrEmptyTitle)
internal/worker/jobs/pr_jobs.gomodified
@@ -86,41 +86,6 @@ func PRMergeability(deps PRJobsDeps) worker.Handler {
86
 	}
86
 	}
87
 }
87
 }
88
 
88
 
89
-// PRMergePayload — enqueued from the POST .../merge handler. Contains
90
-// the actor + chosen method + optional subject/body overrides.
91
-type PRMergePayload struct {
92
-	PRID        int64  `json:"pr_id"`
93
-	ActorUserID int64  `json:"actor_user_id"`
94
-	Method      string `json:"method"`
95
-	Subject     string `json:"subject,omitempty"`
96
-	Body        string `json:"body,omitempty"`
97
-}
98
-
99
-// PRMerge performs the requested merge strategy and updates DB state.
100
-func PRMerge(deps PRJobsDeps) worker.Handler {
101
-	return func(ctx context.Context, raw json.RawMessage) error {
102
-		var p PRMergePayload
103
-		if err := json.Unmarshal(raw, &p); err != nil {
104
-			return worker.PoisonError(fmt.Errorf("bad payload: %w", err))
105
-		}
106
-		if p.PRID == 0 || p.Method == "" {
107
-			return worker.PoisonError(errors.New("missing pr_id or method"))
108
-		}
109
-		gitDir, err := resolveGitDirForPR(ctx, deps.Pool, deps.RepoFS, p.PRID)
110
-		if err != nil {
111
-			return err
112
-		}
113
-		return pulls.Merge(ctx, pulls.Deps{Pool: deps.Pool, Logger: deps.Logger}, pulls.MergeParams{
114
-			PRID:        p.PRID,
115
-			ActorUserID: p.ActorUserID,
116
-			GitDir:      gitDir,
117
-			Method:      p.Method,
118
-			Subject:     p.Subject,
119
-			Body:        p.Body,
120
-		})
121
-	}
122
-}
123
-
124
 // resolveGitDirForPR turns a PR id into the bare-repo path on disk
89
 // resolveGitDirForPR turns a PR id into the bare-repo path on disk
125
 // using the repo + owner-user lookups. Cached per-call only; the
90
 // using the repo + owner-user lookups. Cached per-call only; the
126
 // caller's job-level context bounds the work.
91
 // caller's job-level context bounds the work.
internal/worker/types.gomodified
@@ -37,12 +37,15 @@ const (
37
 )
37
 )
38
 
38
 
39
 // S22 pull-request kinds. Synchronize refreshes commits + files after
39
 // S22 pull-request kinds. Synchronize refreshes commits + files after
40
-// a head-side push; mergeability runs the merge-tree probe; merge
40
+// a head-side push; mergeability runs the merge-tree probe.
41
-// performs the requested strategy in a temp worktree.
41
+//
42
+// (No async-merge kind: the POST .../merge handler executes the merge
43
+// synchronously so the redirect lands on the merged state. If we add
44
+// async merging later — for very large repos — re-introduce a
45
+// KindPRMerge here and a matching jobs.PRMerge handler.)
42
 const (
46
 const (
43
-	KindPRSynchronize Kind = "pr:synchronize"
47
+	KindPRSynchronize  Kind = "pr:synchronize"
44
 	KindPRMergeability Kind = "pr:mergeability"
48
 	KindPRMergeability Kind = "pr:mergeability"
45
-	KindPRMerge        Kind = "pr:merge"
46
 )
49
 )
47
 
50
 
48
 // NotifyChannel is the Postgres LISTEN/NOTIFY channel the pool subscribes
51
 // NotifyChannel is the Postgres LISTEN/NOTIFY channel the pool subscribes