Follow-up from #27711 (CRF-14).
Problem
Five configured git provider types (bitbucket-cloud, bitbucket-server, azure-devops, azure-devops-entra, gitea) get (nil, nil) from gitprovider.New, so gitsync re-queues every affected row every DiffStatusTTL (120s) forever. It logs "no provider for origin %q" (coderd/x/gitsync/gitsync.go, the provider == nil branch), blaming origin matching (a config mistake the operator can fix) rather than a missing implementation (which they cannot).
Rows are per-chat (chat_diff_statuses.chat_id is the primary key), not per-commit. The cost is one DB write per affected chat per 120s, multiplied across deployments. Bitbucket and Azure DevOps are used by real paying customers, so the churn is real.
Agreed design decisions (from discussion on #27711)
-
Distinguish "unimplemented git type" from "no matching config." These are two different errors with different root causes. Do not conflate them. A misconfigured origin should keep the current short backoff (the operator can fix it), while an unimplemented type should get a long backoff (nothing to fix until code ships).
-
Long backoff, not a terminal flag. Do not add a permanent unsupported flag or column. A flag would require MarkStale to clear it on revival, which is easy to forget. A long backoff reuses the existing NoTokenBackoff pattern and keeps revival free.
-
Revival is confirmed safe. Worker.MarkStale (coderd/x/gitsync/worker.go) resets stale_at to the past unconditionally via markStaleSingle whenever the agent reports git activity. So a row parked with a long backoff is revived by new commits regardless of the backoff duration. Verified by tracing, not guessing.
-
Migration bump for new providers. When a provider implementation ships (e.g. bitbucket), the accompanying database migration should reset stale_at on the parked rows so they all wake at once. This makes even a very long backoff safe: shipping the implementation ships the wake-up.
Shape of the fix
The distinction is currently collapsed at two layers:
- resolveGitProvider(ctx, origin) returns a bare gitprovider.Provider. A nil conflates "origin matched no config" and "matched a config but the git type is unimplemented." ProviderResolver is func(ctx, origin) gitprovider.Provider, a single return value with no room to carry the reason.
- Config.Git() deliberately returns (nil, nil) for unimplemented types (see CRF-13 and the doc comment in coderd/externalauth/externalauth.go). Callers cannot distinguish the two cases from the return values.
So the fix needs to:
- Signal three states from the resolver: matched+implemented, matched+unimplemented, no-match. Change ProviderResolver to return an error, or add a parallel "is this origin's provider implemented" check.
- Introduce a sentinel such as ErrProviderUnimplemented and propagate it from resolveGitProvider through Refresher.Refresh into the worker's backoff branch (the block next to ErrNoTokenAvailable in Worker.tick).
- Add a long backoff constant (e.g. NotImplementedBackoff) and apply it when the error matches.
- TestWorker_NoTokenBackoff (coderd/x/gitsync/worker_test.go) is the template for the backoff test.
Estimate
About 150-200 lines including tests. Roughly 40 production lines across coderd/exp_chats.go (resolver) and coderd/x/gitsync/gitsync.go / worker.go (type change, propagation, backoff), plus about 100-120 lines of tests. Contained to coderd/x/gitsync and coderd/exp_chats.go. No DB migration is needed for the fix itself; the wake-up migration ships with each future provider implementation.
🤖 Generated by Coder Agents on behalf of @johnstcn.
Follow-up from #27711 (CRF-14).
Problem
Five configured git provider types (bitbucket-cloud, bitbucket-server, azure-devops, azure-devops-entra, gitea) get (nil, nil) from gitprovider.New, so gitsync re-queues every affected row every DiffStatusTTL (120s) forever. It logs "no provider for origin %q" (coderd/x/gitsync/gitsync.go, the provider == nil branch), blaming origin matching (a config mistake the operator can fix) rather than a missing implementation (which they cannot).
Rows are per-chat (chat_diff_statuses.chat_id is the primary key), not per-commit. The cost is one DB write per affected chat per 120s, multiplied across deployments. Bitbucket and Azure DevOps are used by real paying customers, so the churn is real.
Agreed design decisions (from discussion on #27711)
Distinguish "unimplemented git type" from "no matching config." These are two different errors with different root causes. Do not conflate them. A misconfigured origin should keep the current short backoff (the operator can fix it), while an unimplemented type should get a long backoff (nothing to fix until code ships).
Long backoff, not a terminal flag. Do not add a permanent unsupported flag or column. A flag would require MarkStale to clear it on revival, which is easy to forget. A long backoff reuses the existing NoTokenBackoff pattern and keeps revival free.
Revival is confirmed safe. Worker.MarkStale (coderd/x/gitsync/worker.go) resets stale_at to the past unconditionally via markStaleSingle whenever the agent reports git activity. So a row parked with a long backoff is revived by new commits regardless of the backoff duration. Verified by tracing, not guessing.
Migration bump for new providers. When a provider implementation ships (e.g. bitbucket), the accompanying database migration should reset stale_at on the parked rows so they all wake at once. This makes even a very long backoff safe: shipping the implementation ships the wake-up.
Shape of the fix
The distinction is currently collapsed at two layers:
So the fix needs to:
Estimate
About 150-200 lines including tests. Roughly 40 production lines across coderd/exp_chats.go (resolver) and coderd/x/gitsync/gitsync.go / worker.go (type change, propagation, backoff), plus about 100-120 lines of tests. Contained to coderd/x/gitsync and coderd/exp_chats.go. No DB migration is needed for the fix itself; the wake-up migration ships with each future provider implementation.