feat(coderd): refactors github pr sync functionality (#22715)

- Adds `_API_BASE_URL` to `CODER_EXTERNAL_AUTH_CONFIG_`
- Extracts and refactors existing GitHub PR sync logic to new packages
`coderd/gitsync` and `coderd/externalauth/gitprovider`
- Associated wiring and tests

Created using Opus 4.6
This commit is contained in:
Cian Johnston
2026-03-10 18:46:01 +00:00
committed by GitHub
parent cbe46c816e
commit bc27274aba
31 changed files with 4311 additions and 708 deletions
+21
View File
@@ -1539,6 +1539,17 @@ func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.Acquir
return q.db.AcquireProvisionerJob(ctx, arg)
}
func (q *querier) AcquireStaleChatDiffStatuses(ctx context.Context, limitVal int32) ([]database.AcquireStaleChatDiffStatusesRow, error) {
// This is a system-level batch operation used by the gitsync
// background worker. Per-object authorization is impractical
// for a SKIP LOCKED acquisition query; callers must use
// AsChatd context.
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceChat); err != nil {
return nil, err
}
return q.db.AcquireStaleChatDiffStatuses(ctx, limitVal)
}
func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error {
fetch := func(ctx context.Context, arg database.ActivityBumpWorkspaceParams) (database.Workspace, error) {
return q.db.GetWorkspaceByID(ctx, arg.WorkspaceID)
@@ -1577,6 +1588,16 @@ func (q *querier) ArchiveUnusedTemplateVersions(ctx context.Context, arg databas
return q.db.ArchiveUnusedTemplateVersions(ctx, arg)
}
func (q *querier) BackoffChatDiffStatus(ctx context.Context, arg database.BackoffChatDiffStatusParams) error {
// This is a system-level operation used by the gitsync
// background worker to reschedule failed refreshes. Same
// authorization pattern as AcquireStaleChatDiffStatuses.
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceChat); err != nil {
return err
}
return q.db.BackoffChatDiffStatus(ctx, arg)
}
func (q *querier) BatchUpdateWorkspaceAgentMetadata(ctx context.Context, arg database.BatchUpdateWorkspaceAgentMetadataParams) error {
// Could be any workspace agent and checking auth to each workspace agent is overkill for
// the purpose of this function.
+12
View File
@@ -770,6 +770,18 @@ func (s *MethodTestSuite) TestChats() {
dbm.EXPECT().UpsertChatDiffStatusReference(gomock.Any(), arg).Return(diffStatus, nil).AnyTimes()
check.Args(arg).Asserts(chat, policy.ActionUpdate).Returns(diffStatus)
}))
s.Run("AcquireStaleChatDiffStatuses", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
dbm.EXPECT().AcquireStaleChatDiffStatuses(gomock.Any(), int32(10)).Return([]database.AcquireStaleChatDiffStatusesRow{}, nil).AnyTimes()
check.Args(int32(10)).Asserts(rbac.ResourceChat, policy.ActionUpdate).Returns([]database.AcquireStaleChatDiffStatusesRow{})
}))
s.Run("BackoffChatDiffStatus", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
arg := database.BackoffChatDiffStatusParams{
ChatID: uuid.New(),
StaleAt: dbtime.Now(),
}
dbm.EXPECT().BackoffChatDiffStatus(gomock.Any(), arg).Return(nil).AnyTimes()
check.Args(arg).Asserts(rbac.ResourceChat, policy.ActionUpdate).Returns()
}))
s.Run("UpsertChatSystemPrompt", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
dbm.EXPECT().UpsertChatSystemPrompt(gomock.Any(), "").Return(nil).AnyTimes()
check.Args("").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
+16
View File
@@ -136,6 +136,14 @@ func (m queryMetricsStore) AcquireProvisionerJob(ctx context.Context, arg databa
return r0, r1
}
func (m queryMetricsStore) AcquireStaleChatDiffStatuses(ctx context.Context, limitVal int32) ([]database.AcquireStaleChatDiffStatusesRow, error) {
start := time.Now()
r0, r1 := m.s.AcquireStaleChatDiffStatuses(ctx, limitVal)
m.queryLatencies.WithLabelValues("AcquireStaleChatDiffStatuses").Observe(time.Since(start).Seconds())
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "AcquireStaleChatDiffStatuses").Inc()
return r0, r1
}
func (m queryMetricsStore) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error {
start := time.Now()
r0 := m.s.ActivityBumpWorkspace(ctx, arg)
@@ -168,6 +176,14 @@ func (m queryMetricsStore) ArchiveUnusedTemplateVersions(ctx context.Context, ar
return r0, r1
}
func (m queryMetricsStore) BackoffChatDiffStatus(ctx context.Context, arg database.BackoffChatDiffStatusParams) error {
start := time.Now()
r0 := m.s.BackoffChatDiffStatus(ctx, arg)
m.queryLatencies.WithLabelValues("BackoffChatDiffStatus").Observe(time.Since(start).Seconds())
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "BackoffChatDiffStatus").Inc()
return r0
}
func (m queryMetricsStore) BatchUpdateWorkspaceAgentMetadata(ctx context.Context, arg database.BatchUpdateWorkspaceAgentMetadataParams) error {
start := time.Now()
r0 := m.s.BatchUpdateWorkspaceAgentMetadata(ctx, arg)
+29
View File
@@ -103,6 +103,21 @@ func (mr *MockStoreMockRecorder) AcquireProvisionerJob(ctx, arg any) *gomock.Cal
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AcquireProvisionerJob", reflect.TypeOf((*MockStore)(nil).AcquireProvisionerJob), ctx, arg)
}
// AcquireStaleChatDiffStatuses mocks base method.
func (m *MockStore) AcquireStaleChatDiffStatuses(ctx context.Context, limitVal int32) ([]database.AcquireStaleChatDiffStatusesRow, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "AcquireStaleChatDiffStatuses", ctx, limitVal)
ret0, _ := ret[0].([]database.AcquireStaleChatDiffStatusesRow)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// AcquireStaleChatDiffStatuses indicates an expected call of AcquireStaleChatDiffStatuses.
func (mr *MockStoreMockRecorder) AcquireStaleChatDiffStatuses(ctx, limitVal any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AcquireStaleChatDiffStatuses", reflect.TypeOf((*MockStore)(nil).AcquireStaleChatDiffStatuses), ctx, limitVal)
}
// ActivityBumpWorkspace mocks base method.
func (m *MockStore) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error {
m.ctrl.T.Helper()
@@ -161,6 +176,20 @@ func (mr *MockStoreMockRecorder) ArchiveUnusedTemplateVersions(ctx, arg any) *go
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ArchiveUnusedTemplateVersions", reflect.TypeOf((*MockStore)(nil).ArchiveUnusedTemplateVersions), ctx, arg)
}
// BackoffChatDiffStatus mocks base method.
func (m *MockStore) BackoffChatDiffStatus(ctx context.Context, arg database.BackoffChatDiffStatusParams) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "BackoffChatDiffStatus", ctx, arg)
ret0, _ := ret[0].(error)
return ret0
}
// BackoffChatDiffStatus indicates an expected call of BackoffChatDiffStatus.
func (mr *MockStoreMockRecorder) BackoffChatDiffStatus(ctx, arg any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BackoffChatDiffStatus", reflect.TypeOf((*MockStore)(nil).BackoffChatDiffStatus), ctx, arg)
}
// BatchUpdateWorkspaceAgentMetadata mocks base method.
func (m *MockStore) BatchUpdateWorkspaceAgentMetadata(ctx context.Context, arg database.BatchUpdateWorkspaceAgentMetadataParams) error {
m.ctrl.T.Helper()
+2
View File
@@ -39,6 +39,7 @@ type sqlcQuerier interface {
// multiple provisioners from acquiring the same jobs. See:
// https://www.postgresql.org/docs/9.5/sql-select.html#SQL-FOR-UPDATE-SHARE
AcquireProvisionerJob(ctx context.Context, arg AcquireProvisionerJobParams) (ProvisionerJob, error)
AcquireStaleChatDiffStatuses(ctx context.Context, limitVal int32) ([]AcquireStaleChatDiffStatusesRow, error)
// Bumps the workspace deadline by the template's configured "activity_bump"
// duration (default 1h). If the workspace bump will cross an autostart
// threshold, then the bump is autostart + TTL. This is the deadline behavior if
@@ -60,6 +61,7 @@ type sqlcQuerier interface {
// Only unused template versions will be archived, which are any versions not
// referenced by the latest build of a workspace.
ArchiveUnusedTemplateVersions(ctx context.Context, arg ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error)
BackoffChatDiffStatus(ctx context.Context, arg BackoffChatDiffStatusParams) error
BatchUpdateWorkspaceAgentMetadata(ctx context.Context, arg BatchUpdateWorkspaceAgentMetadataParams) error
BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg BatchUpdateWorkspaceLastUsedAtParams) error
BatchUpdateWorkspaceNextStartAt(ctx context.Context, arg BatchUpdateWorkspaceNextStartAtParams) error
+116
View File
@@ -3026,6 +3026,102 @@ func (q *sqlQuerier) AcquireChat(ctx context.Context, arg AcquireChatParams) (Ch
return i, err
}
const acquireStaleChatDiffStatuses = `-- name: AcquireStaleChatDiffStatuses :many
WITH acquired AS (
UPDATE
chat_diff_statuses
SET
-- Claim for 5 minutes. The worker sets the real stale_at
-- after refresh. If the worker crashes, rows become eligible
-- again after this interval.
stale_at = NOW() + INTERVAL '5 minutes',
updated_at = NOW()
WHERE
chat_id IN (
SELECT
cds.chat_id
FROM
chat_diff_statuses cds
INNER JOIN
chats c ON c.id = cds.chat_id
WHERE
cds.stale_at <= NOW()
AND cds.git_remote_origin != ''
AND cds.git_branch != ''
AND c.archived = FALSE
ORDER BY
cds.stale_at ASC
FOR UPDATE OF cds
SKIP LOCKED
LIMIT
$1::int
)
RETURNING chat_id, url, pull_request_state, changes_requested, additions, deletions, changed_files, refreshed_at, stale_at, created_at, updated_at, git_branch, git_remote_origin
)
SELECT
acquired.chat_id, acquired.url, acquired.pull_request_state, acquired.changes_requested, acquired.additions, acquired.deletions, acquired.changed_files, acquired.refreshed_at, acquired.stale_at, acquired.created_at, acquired.updated_at, acquired.git_branch, acquired.git_remote_origin,
c.owner_id
FROM
acquired
INNER JOIN
chats c ON c.id = acquired.chat_id
`
type AcquireStaleChatDiffStatusesRow struct {
ChatID uuid.UUID `db:"chat_id" json:"chat_id"`
Url sql.NullString `db:"url" json:"url"`
PullRequestState sql.NullString `db:"pull_request_state" json:"pull_request_state"`
ChangesRequested bool `db:"changes_requested" json:"changes_requested"`
Additions int32 `db:"additions" json:"additions"`
Deletions int32 `db:"deletions" json:"deletions"`
ChangedFiles int32 `db:"changed_files" json:"changed_files"`
RefreshedAt sql.NullTime `db:"refreshed_at" json:"refreshed_at"`
StaleAt time.Time `db:"stale_at" json:"stale_at"`
CreatedAt time.Time `db:"created_at" json:"created_at"`
UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
GitBranch string `db:"git_branch" json:"git_branch"`
GitRemoteOrigin string `db:"git_remote_origin" json:"git_remote_origin"`
OwnerID uuid.UUID `db:"owner_id" json:"owner_id"`
}
func (q *sqlQuerier) AcquireStaleChatDiffStatuses(ctx context.Context, limitVal int32) ([]AcquireStaleChatDiffStatusesRow, error) {
rows, err := q.db.QueryContext(ctx, acquireStaleChatDiffStatuses, limitVal)
if err != nil {
return nil, err
}
defer rows.Close()
var items []AcquireStaleChatDiffStatusesRow
for rows.Next() {
var i AcquireStaleChatDiffStatusesRow
if err := rows.Scan(
&i.ChatID,
&i.Url,
&i.PullRequestState,
&i.ChangesRequested,
&i.Additions,
&i.Deletions,
&i.ChangedFiles,
&i.RefreshedAt,
&i.StaleAt,
&i.CreatedAt,
&i.UpdatedAt,
&i.GitBranch,
&i.GitRemoteOrigin,
&i.OwnerID,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Close(); err != nil {
return nil, err
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const archiveChatByID = `-- name: ArchiveChatByID :exec
UPDATE chats SET archived = true, updated_at = NOW()
WHERE id = $1 OR root_chat_id = $1
@@ -3036,6 +3132,26 @@ func (q *sqlQuerier) ArchiveChatByID(ctx context.Context, id uuid.UUID) error {
return err
}
const backoffChatDiffStatus = `-- name: BackoffChatDiffStatus :exec
UPDATE
chat_diff_statuses
SET
stale_at = $1::timestamptz,
updated_at = NOW()
WHERE
chat_id = $2::uuid
`
type BackoffChatDiffStatusParams struct {
StaleAt time.Time `db:"stale_at" json:"stale_at"`
ChatID uuid.UUID `db:"chat_id" json:"chat_id"`
}
func (q *sqlQuerier) BackoffChatDiffStatus(ctx context.Context, arg BackoffChatDiffStatusParams) error {
_, err := q.db.ExecContext(ctx, backoffChatDiffStatus, arg.StaleAt, arg.ChatID)
return err
}
const deleteAllChatQueuedMessages = `-- name: DeleteAllChatQueuedMessages :exec
DELETE FROM chat_queued_messages WHERE chat_id = $1
`
+49
View File
@@ -448,3 +448,52 @@ LIMIT
-- name: GetChatByIDForUpdate :one
SELECT * FROM chats WHERE id = @id::uuid FOR UPDATE;
-- name: AcquireStaleChatDiffStatuses :many
WITH acquired AS (
UPDATE
chat_diff_statuses
SET
-- Claim for 5 minutes. The worker sets the real stale_at
-- after refresh. If the worker crashes, rows become eligible
-- again after this interval.
stale_at = NOW() + INTERVAL '5 minutes',
updated_at = NOW()
WHERE
chat_id IN (
SELECT
cds.chat_id
FROM
chat_diff_statuses cds
INNER JOIN
chats c ON c.id = cds.chat_id
WHERE
cds.stale_at <= NOW()
AND cds.git_remote_origin != ''
AND cds.git_branch != ''
AND c.archived = FALSE
ORDER BY
cds.stale_at ASC
FOR UPDATE OF cds
SKIP LOCKED
LIMIT
@limit_val::int
)
RETURNING *
)
SELECT
acquired.*,
c.owner_id
FROM
acquired
INNER JOIN
chats c ON c.id = acquired.chat_id;
-- name: BackoffChatDiffStatus :exec
UPDATE
chat_diff_statuses
SET
stale_at = @stale_at::timestamptz,
updated_at = NOW()
WHERE
chat_id = @chat_id::uuid;