|
| 1 | +# Version Control Phase 1: VCS Driver Foundation |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +Introduce a provider-neutral VCS layer and rewrite the local Git implementation as an Effect-native driver. This phase should preserve user-visible behavior while replacing the Git-first service boundary with an abstraction that can support Git, Jujutsu, and later Sapling or another viable VCS. |
| 6 | + |
| 7 | +The existing `GitCore` implementation is a behavior reference and source of regression tests, not the target architecture. New code should follow the newer package style used by `effect-acp` and `effect-codex-app-server`: typed service tags, schema-backed tagged errors, scoped process usage, explicit decode boundaries, and no Promise-based process helper as the core execution primitive. |
| 8 | + |
| 9 | +## Scope |
| 10 | + |
| 11 | +- Add VCS-domain contracts in `packages/contracts/src/vcs.ts`. |
| 12 | +- Add shared runtime parsing helpers in `packages/shared/src/vcs/*` only when they are useful to both server and web. |
| 13 | +- Add server services under `apps/server/src/vcs`: |
| 14 | + - `Services/VcsDriver.ts` |
| 15 | + - `Services/VcsRepositoryResolver.ts` |
| 16 | + - `Services/VcsProcess.ts` |
| 17 | + - `Layers/GitVcsDriver.ts` |
| 18 | + - `errors.ts` |
| 19 | +- Migrate server callers from Git-specific terms where the operation is actually VCS-generic. |
| 20 | +- Update active consumers to the new VCS APIs in the same phase; do not add backwards-compatible export shims. |
| 21 | +- Leave source-control hosting providers out of this phase except for remote metadata needed to describe repository status. |
| 22 | + |
| 23 | +## Non-Goals |
| 24 | + |
| 25 | +- No GitLab, Azure DevOps, or GitHub provider rewrite yet. |
| 26 | +- No Jujutsu driver yet, but every interface must be designed so a Jujutsu driver does not have to pretend to be Git. |
| 27 | +- No T3 Review implementation yet. |
| 28 | +- No broad UI redesign. |
| 29 | + |
| 30 | +## Driver Model |
| 31 | + |
| 32 | +Use provider-neutral nouns in new APIs: |
| 33 | + |
| 34 | +- `VcsDriver`: local repository mechanics. |
| 35 | +- `RepositoryIdentity`: detected VCS kind, root path, common metadata path when available, remotes. |
| 36 | +- `WorkingCopyStatus`: dirty state, changed files, aggregate insertions/deletions, current branch/bookmark/change name. |
| 37 | +- `ChangeSet`: a committed or pending unit of change, not necessarily a Git commit. |
| 38 | +- `RefName`: branch, bookmark, tag, or provider-specific ref. |
| 39 | + |
| 40 | +The initial driver capabilities should be explicit: |
| 41 | + |
| 42 | +```ts |
| 43 | +export interface VcsDriverCapabilities { |
| 44 | + readonly kind: "git" | "jj" | "sapling" | "unknown"; |
| 45 | + readonly supportsWorktrees: boolean; |
| 46 | + readonly supportsBookmarks: boolean; |
| 47 | + readonly supportsAtomicSnapshot: boolean; |
| 48 | + readonly supportsPushDefaultRemote: boolean; |
| 49 | +} |
| 50 | +``` |
| 51 | + |
| 52 | +Do not model Jujutsu as `GitCoreShape extends ...`. The Git driver can expose Git-specific implementation details internally, but the public VCS layer should describe operations by intent: |
| 53 | + |
| 54 | +- `detectRepository(cwd)` |
| 55 | +- `status(cwd, options)` |
| 56 | +- `listRefs(cwd, query/pagination)` |
| 57 | +- `checkoutRef(cwd, ref)` |
| 58 | +- `createRef(cwd, ref, from?)` |
| 59 | +- `createWorkspace(cwd, ref, path?)` |
| 60 | +- `removeWorkspace(path)` |
| 61 | +- `prepareChangeContext(cwd, filePaths?)` |
| 62 | +- `createChange(cwd, message, options)` |
| 63 | +- `push(cwd, target?)` |
| 64 | +- `rangeContext(cwd, base, head)` |
| 65 | +- `listWorkspaceFiles(cwd, options)` |
| 66 | + |
| 67 | +## Effect Process Layer |
| 68 | + |
| 69 | +Create a small reusable `VcsProcess` service instead of using `runProcess`. |
| 70 | + |
| 71 | +Requirements: |
| 72 | + |
| 73 | +- Implement with `ChildProcess` and `ChildProcessSpawner` from `effect/unstable/process`. |
| 74 | +- Support scoped acquisition/release for long-running commands and interruption. |
| 75 | +- Support bounded stdout/stderr collection with truncation markers. |
| 76 | + - DO not eagerly consume full stdout/stderr, return stream apis and expose helpers for consumers so we don't consume streams to memory unnecessarily... |
| 77 | +- Support stdin. |
| 78 | +- Support timeout through Effect scheduling/interruption, not ad-hoc timers. |
| 79 | +- Stream output lines to progress callbacks as Effects. |
| 80 | +- Return a typed `ProcessOutput` value for successful execution. |
| 81 | +- Fail with typed errors, not generic thrown exceptions. |
| 82 | + |
| 83 | +Errors should be schema-backed tagged classes, for example: |
| 84 | + |
| 85 | +- `VcsProcessSpawnError` |
| 86 | +- `VcsProcessExitError` |
| 87 | +- `VcsProcessTimeoutError` |
| 88 | +- `VcsOutputDecodeError` |
| 89 | +- `VcsRepositoryDetectionError` |
| 90 | +- `VcsUnsupportedOperationError` |
| 91 | + |
| 92 | +Every error should carry operation name, command display string, cwd when applicable, exit code when applicable, stderr/stdout tails when useful, and original cause where available. Override `message` for user readable messages that provides meaning and hints where appropriate. Errors are schema backed so the full error details will be persisted and serialized properly when stored to DB/Logfiles. |
| 93 | + |
| 94 | +## Git Driver Rewrite |
| 95 | + |
| 96 | +Rewrite Git support against `VcsProcess`. |
| 97 | + |
| 98 | +Carry forward current behavior from: |
| 99 | + |
| 100 | +- `apps/server/src/git/Layers/GitCore.ts` |
| 101 | +- `apps/server/src/git/Layers/GitCore.test.ts` |
| 102 | +- current Git status/branch/worktree contracts |
| 103 | + |
| 104 | +But split the implementation into smaller modules: |
| 105 | + |
| 106 | +- command execution and hardening config |
| 107 | +- repository detection |
| 108 | +- status parsing |
| 109 | +- branch/ref parsing |
| 110 | +- worktree operations |
| 111 | +- commit/range context generation |
| 112 | +- push/pull operations |
| 113 | + |
| 114 | +Keep parsing deterministic. Prefer Git porcelain formats, null-separated output, and schema decoding for JSON-like command output. Avoid regex parsing where Git gives a structured format. |
| 115 | + |
| 116 | +## Freshness and Local Caching |
| 117 | + |
| 118 | +Define freshness rules in the VCS layer before adding more providers. Local VCS status is cheap enough to refresh often; network-backed status is not. |
| 119 | + |
| 120 | +Treat these as live/local: |
| 121 | + |
| 122 | +- repository detection for the active cwd |
| 123 | +- working copy dirty state |
| 124 | +- staged/unstaged/untracked file summaries |
| 125 | +- current branch/bookmark/change name |
| 126 | +- local branch/bookmark lists |
| 127 | +- local worktree/workspace lists |
| 128 | + |
| 129 | +These may run on user-visible polling, but should still be debounced and coalesced per repository root. Prefer filesystem-triggered invalidation where available, with a short fallback poll interval. Concurrent requests for the same repository/status shape should share one in-flight Effect. |
| 130 | + |
| 131 | +Treat these as cached or explicit-refresh only: |
| 132 | + |
| 133 | +- remote tracking branch refreshes |
| 134 | +- ahead/behind counts that require network fetches |
| 135 | +- default branch discovery from a remote provider |
| 136 | +- remote branch lists beyond locally known refs |
| 137 | + |
| 138 | +The VCS driver should expose freshness metadata with status results: |
| 139 | + |
| 140 | +```ts |
| 141 | +export interface VcsFreshness { |
| 142 | + readonly source: "live-local" | "cached-local" | "cached-remote" | "explicit-remote"; |
| 143 | + readonly observedAt: string; |
| 144 | + readonly expiresAt?: string; |
| 145 | +} |
| 146 | +``` |
| 147 | + |
| 148 | +Remote refreshes should be opt-in per operation, for example `refresh: "local-only" | "allow-cached-remote" | "force-remote"`. The default for background status should be `local-only`. |
| 149 | + |
| 150 | +Use Effect `Cache` for repository identity and expensive local metadata: |
| 151 | + |
| 152 | +- key by resolved repository root plus VCS kind |
| 153 | +- invalidate on cwd/root changes and workspace mutation operations |
| 154 | +- use short TTLs for local status caches when filesystem events are unavailable |
| 155 | +- never hide command failures behind stale values unless the caller explicitly accepts stale data |
| 156 | + |
| 157 | +## Cutover Policy |
| 158 | + |
| 159 | +Prefer direct migration and deletion over compatibility wrappers. |
| 160 | + |
| 161 | +Rules: |
| 162 | + |
| 163 | +- Update consumers to call `VcsDriver`/`VcsRepositoryResolver` directly as soon as the new API exists. |
| 164 | +- Delete migrated `GitCore` service methods and tests in the same PR that moves their consumers. |
| 165 | +- Do not keep backwards-compatible export shims, barrel aliases, or old service names for convenience. |
| 166 | +- Transitional modules are allowed only when a caller group is too complex or risky to migrate in the same PR. |
| 167 | +- Every transitional module must have a narrow owner, a removal checklist, and a test proving it delegates to the new implementation. |
| 168 | +- No new feature work may depend on transitional modules. |
| 169 | + |
| 170 | +Expected transitional candidates: |
| 171 | + |
| 172 | +- The highest-level `GitManager` orchestration can be migrated in slices if doing the full Commit + PR flow in one PR is too risky. |
| 173 | +- WebSocket payload compatibility can remain only where changing it would require a coordinated UI/server protocol migration. Internal server code should still use the new VCS contracts. |
| 174 | + |
| 175 | +## Tests |
| 176 | + |
| 177 | +Add integration-style tests with real temporary Git repositories for the new Git driver: |
| 178 | + |
| 179 | +- non-repository detection |
| 180 | +- status for clean/dirty/untracked/staged states |
| 181 | +- branch/ref list with pagination |
| 182 | +- checkout/create branch |
| 183 | +- worktree create/remove |
| 184 | +- commit context generation with file filters |
| 185 | +- commit creation with hook progress events |
| 186 | +- push behavior against a local bare remote |
| 187 | +- status polling does not perform remote network refresh by default |
| 188 | +- concurrent duplicate status requests are coalesced |
| 189 | +- bounded output/truncation |
| 190 | +- timeout/interruption |
| 191 | +- typed error shape for command failure and missing executable |
| 192 | + |
| 193 | +Move or duplicate only the tests needed to prove behavior, then delete the old service tests in the same migration slice. |
| 194 | + |
| 195 | +## Migration Steps |
| 196 | + |
| 197 | +1. Add `vcs` contracts and tagged errors. |
| 198 | +2. Add `VcsProcess` and unit tests around process execution semantics. |
| 199 | +3. Add `VcsDriver` and `VcsRepositoryResolver` service contracts. |
| 200 | +4. Implement `GitVcsDriver` with real Git command integration tests. |
| 201 | +5. Move `GitStatusBroadcaster` and branch/worktree flows to the VCS service directly. |
| 202 | +6. Move commit/range/push callers to the VCS service directly. |
| 203 | +7. Delete migrated `GitCore` internals and tests as each caller group moves. |
| 204 | +8. Add a transitional adapter only for any remaining `GitManager` path that is explicitly too complex to cut over safely in one PR. |
| 205 | +9. Remove every transitional adapter before starting Phase 2 unless the adapter is documented as blocking on the provider cutover. |
| 206 | + |
| 207 | +## Acceptance Criteria |
| 208 | + |
| 209 | +- Current Git branch/status/worktree/commit behavior remains intact. |
| 210 | +- New Git implementation does not depend on `processRunner.ts`. |
| 211 | +- New errors are typed and inspectable by tests. |
| 212 | +- VCS interfaces contain no GitHub/GitLab/Azure concepts. |
| 213 | +- Active consumers use the new VCS APIs directly; any remaining transitional module has a written removal checklist and no compatibility export shim. |
| 214 | +- Background status refresh is local-only by default and cannot hit provider rate limits. |
| 215 | +- Jujutsu can be added by implementing a real driver instead of conforming to Git command semantics. |
| 216 | +- `bun fmt`, `bun lint`, and `bun typecheck` pass. |
0 commit comments