feat: add GitLab git provider - #25396

Well-structured GitLab provider with strong VCR-based testing infrastructure, clean nested-group handling, and correct SCP remote parsing. The implementation is proportional to the problem and the test-to-code ratio is solid. Pariston tried to build a case that the wrong framing was chosen and couldn't: "Framing #1 (missing implementation) is correct."

Severity count: 1 P2, 8 P3, 5 Nit.

The P2 is a memory safety gap: FetchPullRequestDiff reads the entire HTTP response body into memory before the MaxDiffSize check runs, while the GitHub provider bounds reads with io.LimitReader. This is a library constraint, not a logic error, but it removes the memory cap that the package contract implies.

Several findings converge on silent error handling: resolveExternalAuth swallows Config.Git() constructor errors (9 reviewers flagged this independently), and three zero-valued fields (Additions, Deletions, Commits) are stored as known-valid in the database without being tracked in CODAGT-440.

Two test gaps: CompareTimeout (added as a P2 review fix) has no test, and the FetchBranchDiff integration test has a leniency guard that silently passes in VCR replay mode.

🤖 This review was automatically generated with Coder Agents.

Strong progress. 11 of 13 R1 posted findings addressed, all fixes verified by the panel. The P2 unbounded-read fix (DEREM-3) is correct and matches the GitHub pattern. The commit stats population (DEREM-6) is a genuine improvement. The error logging (DEREM-5) makes misconfiguration diagnosable. Chopper on the fix quality: "Error handling is thorough. Every library call goes through wrapError."

Severity count (new this round): 1 P2, 2 P3, 2 Nit.

The P2 is a regression from the DEREM-3 fix: switching to raw HTTP for FetchPullRequestDiff lost the library's *gitlab.ErrorResponse wrapping, so wrapError can no longer detect 429/403 rate limits on that path. The gitsync worker relies on *RateLimitError to back off; without it, a rate-limited GitLab instance gets hammered. Kite independently confirmed this and provided a concrete fix.

R1 contested findings: DEREM-7 (hardcoded "github" in db2sdk) was closed by the panel (4/7 accept), though 3 reviewers noted that a deferral without a ticket is a drop. This needs a human decision: file a ticket or explicitly accept the gap. DEREM-15 (context.Background) was closed 7/7; the author's defense was verified.

🤖 This review was automatically generated with Coder Agents.

Pull request overview

Adds a GitLab implementation for coderd/externalauth/gitprovider so chat diff/gitsync flows can resolve GitLab repositories, merge requests, and diffs.

Changes:

• Introduces GitLab provider logic for MR status, diff fetching, branch-to-MR resolution, and URL parsing/building.
• Updates provider construction to return errors and threads context through gitsync provider resolution.
• Adds GitLab unit/integration tests with go-vcr cassettes and supporting dependency/config updates.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 9 comments.

	path := strings.TrimPrefix(u.Path, "/")
	path = strings.TrimSuffix(path, "/")

	// Find "-/merge_requests/NUMBER" in the path.
	const mrMarker = "-/merge_requests/"
	idx := strings.Index(path, mrMarker)
	if idx < 0 {
		return PRRef{}, false
	}

	// Everything before the marker (minus trailing slash) is the project path.
	projPath := path[:idx]

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

All prior findings addressed. The P2 rate-limit regression (DEREM-25) is fixed correctly: FetchPullRequestDiff now checks 403/429 status codes and parses retry headers before the generic error path, matching the GitHub provider's pattern. The oversize test gap (DEREM-28) is filled. The duplicate test (DEREM-26) is removed. Clock injection (DEREM-27) is applied to the existing rate limit tests. Kite on the overall quality: "Well-decomposed PR. The constructor error propagation fix is the right structural change; it forces every caller to handle failure explicitly."

Severity count (new this round): 1 P3, 1 Nit.

The P3 is a test coverage gap in the approval/review-state mapping. The Nit is the same time.Now() pattern in the newly-added 429OnRawDiffEndpoint test that DEREM-27 fixed in its siblings.

Across three rounds, 18 findings were posted, 16 fixed, 2 contested and closed by panel vote. The implementation is solid, well-tested, and proportional to the problem. Chopper: "Error handling and diagnostic signal are solid across the new GitLab provider."

🤖 This review was automatically generated with Coder Agents.

Review blocked. DEREM-31 (P3, FetchPullRequestStatus tests never assert Approved, ReviewerCount, or ChangesRequested) has not been addressed or responded to. The thread was resolved in the GitHub UI without a reply or code change.

DEREM-30 (Nit, time.Now in 429OnRawDiffEndpoint) was fixed in 85e3d3b.

Further review is blocked until DEREM-31 receives a response: fix the test gap, contest the finding with a technical defense, or acknowledge it with a reason.

🤖 This review was automatically generated with Coder Agents.

All prior findings addressed across five rounds. DEREM-31 (approval test assertions) fixed in f642982. The test suite is comprehensive: ~1,400 lines of tests against ~750 lines of implementation, covering edge cases (HeadSHA fallback, oversize diffs, compare timeout, rate limits on both library and raw HTTP paths, self-hosted URL routing), VCR integration tests against four fixture MRs, and newly-added approval field assertions. Mafu-san: "The pattern repeated with DEREM-3, where the fix propagated to FetchBranchDiff too, not just the cited location."

Severity count (new this round): 2 P3.

Both are minor gaps in the new code. CRF-1: ListMergeRequestDiffs fetches one page (100 entries) without pagination, so MRs with 100+ files get undercounted additions/deletions while ChangedFiles is correct. CRF-2: FetchBranchDiff drops the response body on non-200 errors while its sibling FetchPullRequestDiff includes up to 8 KiB of the body for diagnostics.

Across 5 rounds: 20 findings posted, 18 fixed, 2 contested/closed by panel. The implementation is solid and well-tested.

🤖 This review was automatically generated with Coder Agents.