refactor: map VCS providers by operation

[benvinegar]

Summary

• Replace VCS adapter capability/switch dispatch with explicit per-operation handlers.
• Export concrete adapter implementations (GitVcsAdapter, JjVcsAdapter, SaplingVcsAdapter) through the shared VcsAdapter shape.
• Rename adapter-facing command inputs with a common VCS prefix: VcsDiffCommandInput, VcsShowCommandInput, and VcsStashShowCommandInput.
• Remove the hardcoded VcsMode string union and route default/selected providers through the adapter registry.
• Move untracked-file diff synthesis into adapter-owned paths so loaders.ts no longer branches on provider ids.
• Move Git object/index source reads into src/core/vcs/gitSource.ts, leaving fileSource.ts provider-neutral for shared fetcher primitives and filesystem reads.
• Add shared untracked-file helpers for provider implementations and keep the changeset loader provider-neutral.

Validation

• bun run typecheck
• bun test
• bun run test:integration
• bun run test:tty-smoke
• git diff --check

This PR description was generated by Pi using GPT-5

[greptile-apps]

Greptile Summary

This PR refactors the VCS adapter layer from a monolithic loadReview/watchSignature switch-dispatch pattern to an explicit per-operation handler map (operations["working-tree-diff" | "revision-show" | "stash-show"]). Alongside the adapter restructure, it moves Git object-source reads into a new vcs/gitSource.ts, extracts untracked-file synthesis helpers into vcs/untracked.ts, makes loaders.ts provider-neutral, and renames the input types with a Vcs prefix.

• Replaces VcsCapabilities + adapter.loadReview(operation, ctx) switch dispatch with typed VcsOperations map where each key is an optional { load, watchSignature? } handler; adapter support for an operation is now expressed by its presence in the map rather than a capability set.
• Moves untrackedFiles?: string[] from VcsPatchResult to fully built DiffFile entries inside extraFiles, so adapters own the complete untracked-file construction (binary detection, large-file skipping, source fetchers) rather than delegating back to loaders.ts.
• Broadens VcsMode and VcsId from a closed "git" | "jj" | "sl" union to string, enabling dynamic/future adapter registration at the cost of compile-time ID validation.

Confidence Score: 4/5

Safe to merge — the refactoring correctly preserves all three adapters' load and watch-signature behavior, and the untracked-file and large-file paths produce the same output order as before.

The operation-map dispatch, gitSource extraction, and untracked-file helper split are all logically equivalent to the switch-dispatch they replace. The only non-trivial concerns are the stash-show error message now depending on registry order (benign today, fragile under future extension) and the readGitSpec fallback creating a short-lived FileSourceFetcher for non-git specs. Neither affects correctness now.

src/core/vcs/index.ts (stash-show error message registry dependency) and src/core/vcs/gitSource.ts (readGitSpec fallback allocation).

Important Files Changed

Filename	Overview
src/core/vcs/types.ts	Replaces VcsCapabilities + switch-dispatch shape with a typed VcsOperations map; removes untrackedFiles from VcsPatchResult; broadens VcsId to string.
src/core/vcs/index.ts	Adds loadVcsReview, createVcsWatchSignature, getConfiguredVcsAdapter helpers; stash-show error message now dynamically discovers the supporting adapter by registry order rather than hardcoding Git.
src/core/vcs/git.ts	Converts gitAdapter to GitVcsAdapter with per-operation handlers; untracked file synthesis moved inline; large tracked + untracked files unified into extraFiles.
src/core/vcs/gitSource.ts	New file extracting Git object source reads (git-blob, git-index) from fileSource.ts; readGitSpec delegates non-git specs back to createFileSourceFetcher creating a transient wrapper object.
src/core/vcs/untracked.ts	New file containing shared untracked-file helpers extracted from loaders.ts.
src/core/vcs/jj.ts	Converts jjAdapter to JjVcsAdapter with per-operation map; stash-show removed from the map instead of throwing.
src/core/vcs/sl.ts	Converts slAdapter to SaplingVcsAdapter with per-operation map; untracked synthesis delegates to buildFilesystemUntrackedDiffFile.
src/core/loaders.ts	Provider-neutralized: removes per-provider branching and untracked-file construction; delegates entirely to adapter extraFiles.
src/core/fileSource.ts	Git-specific spec kinds removed; logSourceDiagnostic and readStreamTextWithLimit exported; gitExecutable option removed.
src/core/types.ts	Renames input types with Vcs prefix; VcsMode broadened from literal union to string.
src/core/watch.ts	vcsPatchSignature delegates to createVcsWatchSignature; redundant checks removed.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant L as loaders.ts
    participant V as vcs/index.ts
    participant A as VcsAdapter (Git/Jj/Sl)
    participant U as vcs/untracked.ts
    participant G as vcs/gitSource.ts

    L->>V: loadVcsReview(adapter, operation, ctx)
    V->>V: getVcsOperation(adapter, operation)
    V->>A: handler.load(operation.input, ctx)
    A->>U: inspectLargeUntrackedFile / buildGit/FilesystemUntrackedDiffFile
    A->>G: createGitFileSourceFetcher(specs)
    G-->>A: FileSourceFetcher
    A-->>V: "VcsPatchResult { patchText, extraFiles, sourceFetcherBuilder }"
    V-->>L: VcsPatchResult
    L->>L: normalizePatchChangeset(patchText)
    L->>L: "adapterFiles = extraFiles.map(f => { ...f, agent: findAgentFileContext })"
    L-->>L: "Changeset { files: [parsedFiles, ...adapterFiles] }"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant L as loaders.ts
    participant V as vcs/index.ts
    participant A as VcsAdapter (Git/Jj/Sl)
    participant U as vcs/untracked.ts
    participant G as vcs/gitSource.ts

    L->>V: loadVcsReview(adapter, operation, ctx)
    V->>V: getVcsOperation(adapter, operation)
    V->>A: handler.load(operation.input, ctx)
    A->>U: inspectLargeUntrackedFile / buildGit/FilesystemUntrackedDiffFile
    A->>G: createGitFileSourceFetcher(specs)
    G-->>A: FileSourceFetcher
    A-->>V: "VcsPatchResult { patchText, extraFiles, sourceFetcherBuilder }"
    V-->>L: VcsPatchResult
    L->>L: normalizePatchChangeset(patchText)
    L->>L: "adapterFiles = extraFiles.map(f => { ...f, agent: findAgentFileContext })"
    L-->>L: "Changeset { files: [parsedFiles, ...adapterFiles] }"

Loading

Comments Outside Diff (1)

1. src/core/types.ts, line 911 (link)

   VcsMode broadened to string removes compile-time ID validation

   Changing VcsMode from "git" | "jj" | "sl" to string means TypeScript will no longer flag values like vcs: "svn" or vcs: "gt" as errors in CommonOptions. The same applies to VcsId in vcs/types.ts. Runtime protection still exists via getVcsAdapter's throw and isVcsId, but config parsing, CLI option handling, and any exhaustive switch on VcsMode lose the compile-time guard. If the intent is to enable dynamic adapters, a documented type alias (e.g., type VcsMode = string & {}) would at least preserve the original names as documentation while still being assignable to string.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/core/types.ts
   Line: 911
   
   Comment:
   **`VcsMode` broadened to `string` removes compile-time ID validation**
   
   Changing `VcsMode` from `"git" | "jj" | "sl"` to `string` means TypeScript will no longer flag values like `vcs: "svn"` or `vcs: "gt"` as errors in `CommonOptions`. The same applies to `VcsId` in `vcs/types.ts`. Runtime protection still exists via `getVcsAdapter`'s `throw` and `isVcsId`, but config parsing, CLI option handling, and any exhaustive `switch` on `VcsMode` lose the compile-time guard. If the intent is to enable dynamic adapters, a documented type alias (e.g., `type VcsMode = string & {}`) would at least preserve the original names as documentation while still being assignable to `string`.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/core/vcs/gitSource.ts:129-133
**Roundabout delegation for `"none"`/`"fs"` specs**

When `spec.kind` is neither `"git-blob"` nor `"git-index"`, `readGitSpec` constructs a full `FileSourceFetcher` — including an internal `Map` cache — only to immediately call `getFullText("old")` and discard the fetcher. For the `"none"` case this allocates an object just to return `null`; for the `"fs"` case the cached result is never reused. A direct early-return branch (`if (spec.kind === "none") return null; return readFsSpec(spec, maxSourceBytes)`) would eliminate the indirection without changing behaviour. This requires exporting `readFsSpec` from `fileSource.ts`, but `logSourceDiagnostic` and `readStreamTextWithLimit` are already exported in this PR.

### Issue 2 of 3
src/core/vcs/index.ts:137-141
**`stash-show` redirect message is sensitive to adapter registry order**

`vcsAdapters.find(...)` returns the *first* adapter in `[JjVcsAdapter, SaplingVcsAdapter, GitVcsAdapter]` that has a `"stash-show"` entry. Today only `GitVcsAdapter` carries that operation, so the message correctly says "requires Git VCS mode." If `stash-show` were added to `JjVcsAdapter` or `SaplingVcsAdapter` before `GitVcsAdapter` in a future change, the error would suggest using the wrong provider — e.g., "requires Jujutsu VCS mode." — without the user having any Git stash to inspect. The original code hardcoded the Git requirement for stash-show, which was semantically more precise. Consider retaining a direct reference (`GitVcsAdapter.id` / `GitVcsAdapter.name`) or adding a comment explaining the ordering assumption.

### Issue 3 of 3
src/core/types.ts:911
**`VcsMode` broadened to `string` removes compile-time ID validation**

Changing `VcsMode` from `"git" | "jj" | "sl"` to `string` means TypeScript will no longer flag values like `vcs: "svn"` or `vcs: "gt"` as errors in `CommonOptions`. The same applies to `VcsId` in `vcs/types.ts`. Runtime protection still exists via `getVcsAdapter`'s `throw` and `isVcsId`, but config parsing, CLI option handling, and any exhaustive `switch` on `VcsMode` lose the compile-time guard. If the intent is to enable dynamic adapters, a documented type alias (e.g., `type VcsMode = string & {}`) would at least preserve the original names as documentation while still being assignable to `string`.

_{Reviews (1): Last reviewed commit: "refactor: map vcs providers by operation" | Re-trigger Greptile}

[greptile-apps]

Roundabout delegation for "none"/"fs" specs

When spec.kind is neither "git-blob" nor "git-index", readGitSpec constructs a full FileSourceFetcher — including an internal Map cache — only to immediately call getFullText("old") and discard the fetcher. For the "none" case this allocates an object just to return null; for the "fs" case the cached result is never reused. A direct early-return branch (if (spec.kind === "none") return null; return readFsSpec(spec, maxSourceBytes)) would eliminate the indirection without changing behaviour. This requires exporting readFsSpec from fileSource.ts, but logSourceDiagnostic and readStreamTextWithLimit are already exported in this PR.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/core/vcs/gitSource.ts
Line: 129-133

Comment:
**Roundabout delegation for `"none"`/`"fs"` specs**

When `spec.kind` is neither `"git-blob"` nor `"git-index"`, `readGitSpec` constructs a full `FileSourceFetcher` — including an internal `Map` cache — only to immediately call `getFullText("old")` and discard the fetcher. For the `"none"` case this allocates an object just to return `null`; for the `"fs"` case the cached result is never reused. A direct early-return branch (`if (spec.kind === "none") return null; return readFsSpec(spec, maxSourceBytes)`) would eliminate the indirection without changing behaviour. This requires exporting `readFsSpec` from `fileSource.ts`, but `logSourceDiagnostic` and `readStreamTextWithLimit` are already exported in this PR.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

stash-show redirect message is sensitive to adapter registry order

vcsAdapters.find(...) returns the first adapter in [JjVcsAdapter, SaplingVcsAdapter, GitVcsAdapter] that has a "stash-show" entry. Today only GitVcsAdapter carries that operation, so the message correctly says "requires Git VCS mode." If stash-show were added to JjVcsAdapter or SaplingVcsAdapter before GitVcsAdapter in a future change, the error would suggest using the wrong provider — e.g., "requires Jujutsu VCS mode." — without the user having any Git stash to inspect. The original code hardcoded the Git requirement for stash-show, which was semantically more precise. Consider retaining a direct reference (GitVcsAdapter.id / GitVcsAdapter.name) or adding a comment explaining the ordering assumption.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/core/vcs/index.ts
Line: 137-141

Comment:
**`stash-show` redirect message is sensitive to adapter registry order**

`vcsAdapters.find(...)` returns the *first* adapter in `[JjVcsAdapter, SaplingVcsAdapter, GitVcsAdapter]` that has a `"stash-show"` entry. Today only `GitVcsAdapter` carries that operation, so the message correctly says "requires Git VCS mode." If `stash-show` were added to `JjVcsAdapter` or `SaplingVcsAdapter` before `GitVcsAdapter` in a future change, the error would suggest using the wrong provider — e.g., "requires Jujutsu VCS mode." — without the user having any Git stash to inspect. The original code hardcoded the Git requirement for stash-show, which was semantically more precise. Consider retaining a direct reference (`GitVcsAdapter.id` / `GitVcsAdapter.name`) or adding a comment explaining the ordering assumption.

How can I resolve this? If you propose a fix, please make it concise.