Repository navigation
Converge the Git layer onto upstream's driver, GitManager, and broadcaster - #105
Merged
Merged
Conversation
…aster Restore upstream GitVcsDriverCore, GitVcsDriver, GitManager, GitWorkflowService, VcsStatusBroadcaster, and remoteRefs from 024d495, keeping only the Coder seams documented in docs/internals/coder-only.md: - worktree moves, index-preserving ref reads, 32 MiB typed review expansion, single-source review, bounded t3.json reads, and checkpoint identity - GitLab-only process classification and the write-probe hook - a provider write-access gate before any MR action, GitLab MR templates, and refs/merge-requests head fetches - the sidebar ref stream, per-project auto-pull, and a demand-only BackgroundPolicy stand-in Replace coderVcsStatus with the broadcaster, move the MR reactors and storage cleanup onto GitManager, port upstream includeBranch commit/branch generation, and re-export the upstream shared helpers GitManager uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve the overlap with #104: keep the broadcaster-based status wiring in coderWs and add #104's repository-identity refresh after publishing, ahead of the (now detached, as upstream) status refresh. Move the reworked settlement tests onto GitManager. Disable Git's detached auto-maintenance in the shared test Git config so it cannot race scoped temp-repo cleanup (ENOTEMPTY), and point the branch-MR discovery doc at GitManager's cache. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The workspace helper runs where upstream's server runs, so there is no Coder-boundary reason for the fork to replace upstream's Git core. Because it did, every upstream fix in this area had to be found and re-ported by hand, and some were missed. This PR restores upstream's implementations from
024d49520eand keeps only the documented Coder seams.Restored from upstream:
vcs/GitVcsDriver.tsandvcs/GitVcsDriverCore.ts, which replace the fork-writtenmakeLocalGitService. Metrics and span events are stripped.git/GitManager.tsandgit/GitWorkflowService.ts, which replace the fork's ~1,600-lineGitWorkflowService.vcs/VcsStatusBroadcaster.ts, which replacescoderVcsStatus.ts.git/remoteRefs.ts.VcsProcessconcurrency.Coder seams, each marked
Coder:in the code and listed indocs/internals/coder-only.mdunder "Upstream seams":moveWorktree, used when a thread branch rename also moves its worktree folder;refStatusLocal, a branch read that never rewrites the real index;t3.jsonreads;GitManager:refs/merge-requests/<n>/headfetches.subscribeVcsRefStatusstream;BackgroundPolicystand-in that only polls while a status subscription is open;Also in this PR:
ThreadSettlementReactor,ThreadPullRequestReactor, andstorageCleanupnow useGitManager, as upstream does.includeBranchcommit-and-branch generation is ported, so feature branches get generated names again.isProviderAvailable,getChangeRequestTerminologyForKind,isSshRemoteUrl, and the provider-awareresolveSourceControlWriterModelSelectionare re-exported, as upstream exports them.Behavior changes kept for upstream parity:
GitManagerstill emits its fallback completion event after a successful commit; timing and duration can differ.create_prskips the push when the branch is already up to date.git submodule update --init, which may fetch submodules from the network.Phase (d), converging
coderWs.tsonto upstream'sws.tshandlers, will follow as a separate stacked PR.coder-onlyatc646aed1cdis merged into this branch. The conflicts preserve #104's repository-identity refresh after publish and settlement behavior alongside the restored upstream Git services. The stalecoderVcsStatus.test.tsentry is removed, and the merged tree is verified.Follow-up verification also ports upstream's Codex
includeBranchtest and disables detached Git auto-maintenance in temporary test repositories, preventing theENOTEMPTYcleanup race without changing production Git behavior. A live GitLab run found that the Coder write probe neededglab api --include: GitLab can reject missing MR fields with HTTP 400 before checking the impossible project ID, and response headers supply the required GitLab fingerprint. The classifier still rejects generic proxy responses; a regression test covers this real response.Testing
pnpm test:server: 185 files, 2,133 tests passing, including the write-probe regression.pnpm buildpasses.GitVcsDriverCore.test.ts,GitVcsDriver.test.ts,GitManager.test.ts(GitLab replacesgh),GitWorkflowService.test.ts, andVcsStatusBroadcaster.test.ts.gitand scriptedglab,claude, andssh, comparing this branch with the original fork. Every remaining difference is intentional upstream behavior. The harness caught two bugs, both fixed here: the missingincludeBranchsupport and a write check that only ran for remotes detected as GitLab.