fix(coderd): fast-retry PR discovery after git push (#23579)

## Problem

When chatd pushes a branch and then creates a PR (e.g. `git push`
followed by `gh pr create`), the gitsync background worker often picks
up the stale `chat_diff_statuses` row between the two operations. At
that point no PR exists yet, so the worker skips the row. However, the
acquisition SQL locks the row for **5 minutes** (crash-recovery
interval), creating a dead zone where the PR diff is invisible in the UI
until the user manually navigates to the chat.

### Root cause

1. `git push` triggers `GIT_ASKPASS` → coderd external-auth handler →
`MarkStale()` sets `stale_at = now - 1s`
2. Background worker acquires the row within ~10s, atomically bumps
`stale_at = NOW() + 5 min` (crash-recovery lock)
3. Worker calls `ResolveBranchPullRequest` → no PR exists yet → returns
`nil` → worker skips with `continue`
4. `gh pr create` completes moments later, but uses its own auth (not
`GIT_ASKPASS`), so no second `MarkStale` fires
5. Row is locked for 5 minutes before the worker can retry

Loading the chat works immediately because `GET /chats/{chat}` calls
`resolveChatDiffStatus` synchronously, which discovers the PR inline.

## Fix

When `ResolveBranchPullRequest` returns nil (no PR yet) **and** the row
was recently marked stale (within 2 minutes), apply a short 15-second
backoff via `BackoffChatDiffStatus` instead of letting the 5-minute
acquisition lock stand. Outside the retry window, the worker skips the
row as before — no indefinite fast-polling for branches that never
receive a PR.

To make the "recently marked stale" check work, `updated_at` is no
longer overwritten by the acquisition and backoff SQL queries. This
preserves it as a reliable "last externally changed" timestamp (set by
`MarkStale` or a successful refresh).

### Behavior summary

| Scenario | `updated_at` age | Backoff | Effective retry |
|---|---|---|---|
| Fresh push, no PR yet | < 2 min | 15s (`NoPRBackoff`) | ~15s |
| Old row, no PR | ≥ 2 min | None (skip) | ~5 min (acquisition lock) |
| Error (any age) | Any | 120s (`DiffStatusTTL`) | ~120s |
| Success (any age) | Any | 120s (`DiffStatusTTL`) | ~120s |

## Changes

- **`coderd/database/queries/chats.sql`** — Remove `updated_at = NOW()`
from `AcquireStaleChatDiffStatuses` and `BackoffChatDiffStatus`
- **`coderd/database/queries.sql.go`** — Regenerated
- **`coderd/x/gitsync/worker.go`** — Add `NoPRBackoff` (15s) and
`NoPRRetryWindow` (2 min) constants; apply short backoff only within the
retry window
- **`coderd/x/gitsync/worker_test.go`** — Add
`TestWorker_NoPR_RecentMarkStale_BacksOffShort` and
`TestWorker_NoPR_OldRow_Skips`

coderd/database/queries.sql.go

@@ -3883,8 +3883,11 @@ WITH acquired AS (
         -- 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()
+        -- NOTE: updated_at is intentionally NOT touched here so
+        -- the worker can read it as "when was this row last
+        -- externally changed" (by MarkStale or a successful
+        -- refresh).
+        stale_at = NOW() + INTERVAL '5 minutes'
     WHERE
         chat_id IN (
             SELECT
@@ -4005,8 +4008,11 @@ const backoffChatDiffStatus = `-- name: BackoffChatDiffStatus :exec
 UPDATE
     chat_diff_statuses
 SET
-    stale_at = $1::timestamptz,
-    updated_at = NOW()
+    -- NOTE: updated_at is intentionally NOT touched here so
+    -- the worker can read it as "when was this row last
+    -- externally changed" (by MarkStale or a successful
+    -- refresh).
+    stale_at = $1::timestamptz
 WHERE
     chat_id = $2::uuid
 `

coderd/database/queries/chats.sql

@@ -543,8 +543,11 @@ WITH acquired AS (
         -- 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()
+        -- NOTE: updated_at is intentionally NOT touched here so
+        -- the worker can read it as "when was this row last
+        -- externally changed" (by MarkStale or a successful
+        -- refresh).
+        stale_at = NOW() + INTERVAL '5 minutes'
     WHERE
         chat_id IN (
             SELECT
@@ -579,8 +582,11 @@ INNER JOIN
 UPDATE
     chat_diff_statuses
 SET
-    stale_at = @stale_at::timestamptz,
-    updated_at = NOW()
+    -- NOTE: updated_at is intentionally NOT touched here so
+    -- the worker can read it as "when was this row last
+    -- externally changed" (by MarkStale or a successful
+    -- refresh).
+    stale_at = @stale_at::timestamptz
 WHERE
     chat_id = @chat_id::uuid;
 

coderd/x/gitsync/worker.go

@@ -32,6 +32,24 @@ const (
 	// than DiffStatusTTL because the user must manually link
 	// their account before retrying is useful.
 	NoTokenBackoff = 10 * time.Minute
+
+	// NoPRBackoff is the backoff applied when a branch has no
+	// associated pull request yet. Kept short so that PRs created
+	// shortly after a push (e.g. via `gh pr create`) are
+	// discovered quickly instead of waiting for the 5-minute
+	// acquisition lock to expire.
+	NoPRBackoff = 15 * time.Second
+
+	// NoPRRetryWindow is how long after MarkStale the worker
+	// applies the short NoPRBackoff. Outside this window the
+	// worker lets the 5-minute acquisition lock serve as the
+	// natural retry interval, avoiding indefinite fast-polling
+	// for branches that never receive a PR.
+	//
+	// Together with NoPRBackoff this bounds the number of
+	// GitHub API calls to ~NoPRRetryWindow/NoPRBackoff (≈8)
+	// per push. Keep both values in sync when adjusting.
+	NoPRRetryWindow = 2 * time.Minute
 )
 
 // Store is the narrow DB interface the Worker needs.
@@ -218,7 +236,26 @@ func (w *Worker) tick(ctx context.Context) {
 			continue
 		}
 		if res.Params == nil {
-			// No PR yet — skip.
+			// No PR exists yet for this branch. If the row was
+			// recently marked stale (e.g. a git push just
+			// happened), apply a short backoff so the PR is
+			// discovered quickly once created. Outside the
+			// retry window, do not shorten the backoff; the
+			// 5-minute acquisition lock will serve as the retry
+			// interval instead.
+			age := w.clock.Now().Sub(res.Request.Row.UpdatedAt)
+			if age < NoPRRetryWindow {
+				if err := w.store.BackoffChatDiffStatus(ctx,
+					database.BackoffChatDiffStatusParams{
+						ChatID:  res.Request.Row.ChatID,
+						StaleAt: w.clock.Now().UTC().Add(NoPRBackoff),
+					},
+				); err != nil {
+					w.logger.Warn(ctx, "backoff no-pr chat diff status",
+						slog.F("chat_id", res.Request.Row.ChatID),
+						slog.Error(err))
+				}
+			}
 			continue
 		}
 		if _, err := w.store.UpsertChatDiffStatus(ctx, *res.Params); err != nil {

coderd/x/gitsync/worker_test.go

@@ -221,30 +221,81 @@ func TestWorker_LimitsToNRows(t *testing.T) {
 	assert.Equal(t, int32(numRows), upsertCount.Load())
 }
 
-func TestWorker_RefresherReturnsNilNil_SkipsUpsert(t *testing.T) {
+func TestWorker_NoPR_RecentMarkStale_BacksOffShort(t *testing.T) {
 	t.Parallel()
 	ctx := testutil.Context(t, testutil.WaitShort)
 
 	chatID := uuid.New()
 	ownerID := uuid.New()
 
-	// When the Refresher returns (nil, nil) the worker skips the
-	// upsert and publish. We signal tickDone from the refresher
-	// mock since that is the last operation before the tick
-	// returns.
+	// When the Refresher returns (nil, nil) AND the row was
+	// recently marked stale (updated_at within NoPRRetryWindow),
+	// the worker should call BackoffChatDiffStatus with NoPRBackoff
+	// so the row is retried quickly.
 	tickDone := make(chan struct{})
 
 	ctrl := gomock.NewController(t)
 	store := dbmock.NewMockStore(ctrl)
 
-	store.EXPECT().AcquireStaleChatDiffStatuses(gomock.Any(), gomock.Any()).
-		Return([]database.AcquireStaleChatDiffStatusesRow{makeAcquiredRowWithBranch(chatID, ownerID, "feature")}, nil)
-
 	mClock := quartz.NewMock(t)
+
+	row := makeAcquiredRowWithBranch(chatID, ownerID, "feature")
+	row.UpdatedAt = mClock.Now() // recently marked stale
+
+	store.EXPECT().AcquireStaleChatDiffStatuses(gomock.Any(), gomock.Any()).
+		Return([]database.AcquireStaleChatDiffStatusesRow{row}, nil)
+	store.EXPECT().BackoffChatDiffStatus(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(_ context.Context, arg database.BackoffChatDiffStatusParams) error {
+			assert.Equal(t, chatID, arg.ChatID)
+			expected := mClock.Now().UTC().Add(gitsync.NoPRBackoff)
+			assert.WithinDuration(t, expected, arg.StaleAt, time.Second,
+				"stale_at should be NoPRBackoff from now")
+			close(tickDone)
+			return nil
+		})
+
 	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
 
 	// ResolveBranchPullRequest returns nil → Refresher returns
 	// (nil, nil).
+	refresher := newTestRefresher(t, mClock, withResolveBranchPR(
+		func(context.Context, string, gitprovider.BranchRef) (*gitprovider.PRRef, error) {
+			return nil, nil
+		},
+	))
+
+	worker := gitsync.NewWorker(store, refresher, nil, mClock, logger)
+
+	tickOnce(ctx, t, mClock, worker, tickDone)
+}
+
+func TestWorker_NoPR_OldRow_Skips(t *testing.T) {
+	t.Parallel()
+	ctx := testutil.Context(t, testutil.WaitShort)
+
+	chatID := uuid.New()
+	ownerID := uuid.New()
+
+	// When the Refresher returns (nil, nil) but the row's
+	// updated_at is outside the NoPRRetryWindow, the worker should
+	// skip the row entirely (no backoff call) and let the 5-minute
+	// acquisition lock serve as the natural retry interval.
+	tickDone := make(chan struct{})
+
+	ctrl := gomock.NewController(t)
+	store := dbmock.NewMockStore(ctrl)
+
+	mClock := quartz.NewMock(t)
+
+	row := makeAcquiredRowWithBranch(chatID, ownerID, "feature")
+	row.UpdatedAt = mClock.Now().Add(-5 * time.Minute) // old row
+
+	store.EXPECT().AcquireStaleChatDiffStatuses(gomock.Any(), gomock.Any()).
+		Return([]database.AcquireStaleChatDiffStatusesRow{row}, nil)
+	// BackoffChatDiffStatus should NOT be called.
+
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
 	refresher := newTestRefresher(t, mClock, withResolveBranchPR(
 		func(context.Context, string, gitprovider.BranchRef) (*gitprovider.PRRef, error) {
 			close(tickDone)
@@ -257,6 +308,101 @@ func TestWorker_RefresherReturnsNilNil_SkipsUpsert(t *testing.T) {
 	tickOnce(ctx, t, mClock, worker, tickDone)
 }
 
+func TestWorker_NoPR_BoundaryExactWindow_Skips(t *testing.T) {
+	t.Parallel()
+	ctx := testutil.Context(t, testutil.WaitShort)
+
+	chatID := uuid.New()
+	ownerID := uuid.New()
+
+	// When updated_at is exactly NoPRRetryWindow ago, the strict
+	// "<" comparison means the row should be skipped (no backoff).
+	// This pins the boundary so an accidental change to "<=" is
+	// caught.
+	tickDone := make(chan struct{})
+
+	ctrl := gomock.NewController(t)
+	store := dbmock.NewMockStore(ctrl)
+
+	mClock := quartz.NewMock(t)
+
+	row := makeAcquiredRowWithBranch(chatID, ownerID, "feature")
+	row.UpdatedAt = mClock.Now().Add(-gitsync.NoPRRetryWindow) // exactly at boundary
+
+	store.EXPECT().AcquireStaleChatDiffStatuses(gomock.Any(), gomock.Any()).
+		Return([]database.AcquireStaleChatDiffStatusesRow{row}, nil)
+	// BackoffChatDiffStatus should NOT be called.
+
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	refresher := newTestRefresher(t, mClock, withResolveBranchPR(
+		func(context.Context, string, gitprovider.BranchRef) (*gitprovider.PRRef, error) {
+			close(tickDone)
+			return nil, nil
+		},
+	))
+
+	worker := gitsync.NewWorker(store, refresher, nil, mClock, logger)
+
+	tickOnce(ctx, t, mClock, worker, tickDone)
+}
+
+func TestWorker_NoPR_BackoffError_ContinuesNextRow(t *testing.T) {
+	t.Parallel()
+	ctx := testutil.Context(t, testutil.WaitShort)
+
+	chat1 := uuid.New()
+	chat2 := uuid.New()
+	ownerID := uuid.New()
+
+	// Two recent rows, both with no PR. BackoffChatDiffStatus
+	// fails for the first row but the second row should still
+	// be processed (backoff succeeds).
+	var backoffCount atomic.Int32
+	tickDone := make(chan struct{})
+	var closeOnce sync.Once
+
+	ctrl := gomock.NewController(t)
+	store := dbmock.NewMockStore(ctrl)
+
+	mClock := quartz.NewMock(t)
+
+	row1 := makeAcquiredRowWithBranch(chat1, ownerID, "no-pr-1")
+	row1.UpdatedAt = mClock.Now()
+	row2 := makeAcquiredRowWithBranch(chat2, ownerID, "no-pr-2")
+	row2.UpdatedAt = mClock.Now()
+
+	store.EXPECT().AcquireStaleChatDiffStatuses(gomock.Any(), gomock.Any()).
+		Return([]database.AcquireStaleChatDiffStatusesRow{row1, row2}, nil)
+	store.EXPECT().BackoffChatDiffStatus(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(_ context.Context, arg database.BackoffChatDiffStatusParams) error {
+			n := backoffCount.Add(1)
+			if arg.ChatID == chat1 {
+				return fmt.Errorf("simulated backoff error")
+			}
+			// Second call succeeds; both rows processed.
+			if n >= 2 {
+				closeOnce.Do(func() { close(tickDone) })
+			}
+			return nil
+		}).Times(2)
+
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	refresher := newTestRefresher(t, mClock, withResolveBranchPR(
+		func(context.Context, string, gitprovider.BranchRef) (*gitprovider.PRRef, error) {
+			return nil, nil
+		},
+	))
+
+	worker := gitsync.NewWorker(store, refresher, nil, mClock, logger)
+
+	tickOnce(ctx, t, mClock, worker, tickDone)
+
+	assert.Equal(t, int32(2), backoffCount.Load(),
+		"both rows should have attempted backoff")
+}
+
 func TestWorker_RefresherError_BacksOffRow(t *testing.T) {
 	t.Parallel()
 	ctx := testutil.Context(t, testutil.WaitShort)