Skip to content

feat(extensions): make VCS adapters self-contained - #711

Open
benvinegar wants to merge 2 commits into
mainfrom
refactor/vcs-extension-ownership
Open

feat(extensions): make VCS adapters self-contained#711
benvinegar wants to merge 2 commits into
mainfrom
refactor/vcs-extension-ownership

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

Make bundled and third-party VCS adapters cross the same public extension boundary.

  • Move Git, Jujutsu, and Sapling command execution, revision resolution, failure translation, source loading, and tests beside their bundled extensions.
  • Add an app-owned VcsCatalog and inject it through configuration, loading, reload, watch, and session resolution.
  • Support staged repository discovery so a globally/configured/CLI-installed adapter can establish the repository root before repo config and repo-local extensions load.
  • Publish source-too-large results through hunkdiff/extension, remove Git-specific state from the generic load context, and enforce the ownership boundary with source tests.

Before and after

Area Before After
Provider ownership Bundled extension entrypoints delegated to implementations in src/core/vcs Each implementation and its tests live under src/extensions/default/vcs/<provider>
Dependency direction core -> bundled extensions -> core Core is provider-neutral; src/app owns composition
Adapter contract Bundled Git used private source-limit behavior and a Git-specific generic context field Bundled adapters use hunkdiff/extension; source limits are structural public results and Git owns its process dependency
Repository bootstrap Initial root discovery only knew bundled adapters .hunk is provider-independent and staged startup can discover roots through external adapters
Session selection Repository selectors depended on Git-style assumptions Selectors match subdirectories to the nearest registered live-session root
Enforcement Source boundaries only prevented core -> ui Tests prevent core -> extensions, bundled VCS -> core, and private extension API imports

Git-format patch parsing remains shared in core because Git, Jujutsu, and Sapling all use it as a provider-neutral interchange format.

Before/after behavioral parity

I ran temporary differential harnesses against the same repositories through:

  • Before: origin/main@c8ba80a
  • After: 45265ec

The harness normalized only absolute checkout/source paths and generated changeset timestamp IDs. It then compared complete changesets—including patches, file order, metadata, and statistics—plus exact source reads, repository roots, watch-signature lengths/hashes, and translated error types/messages/suggestions.

Backend Tool version Scenarios Result
Git 2.55.0 13 Exact normalized match
Jujutsu 0.41.0 10 Exact normalized match
Sapling 0.2.20260522-084851+1e764c94 10 Exact normalized match

Git coverage

Working tree with and without untracked files, pathspecs, staged changes, staged pathspecs, revision ranges, show, show pathspecs, stash reviews, invalid refs/ranges, and watch signatures. The fixture included text and binary changes, deletion, mode changes, staged-plus-unstaged edits, rename handling, Unicode paths, and large tracked/untracked files.

Jujutsu coverage

Working-copy review with and without the untracked option, filesets, revision review, show, show filesets, unsupported staging, invalid revsets, and watch signatures. The fixture included text and binary changes, deletion, mode changes, rename handling, and Unicode paths.

Sapling coverage

Working-copy review with and without unknown files, pathspecs, revision review, show, show pathspecs, unsupported staging, invalid revsets, and watch signatures. The fixture included text and binary changes, deletion, mode changes, rename handling, Unicode paths, and a large unknown file.

The intentional differences are architectural or additive: external-only repositories can now bootstrap correctly, repository selectors resolve nested paths to the nearest live root, and structural source-too-large answers are cached so adapters are not called repeatedly.

Verification

  • bun run typecheck
  • bun run lint
  • bun run format:check
  • bun run check:docs
  • bun run check:pack
  • bun run test with both jj and sl available — 2,010 passed, 4 skipped, 0 failed
  • Focused Git/JJ/Sapling loader and adapter coverage — 96 passed, 0 failed
  • bun run test:integration101 passed, 0 failed
  • bun run test:tty-smoke9 passed, 0 failed
  • bun run build:bin

This PR description was generated by Pi using gpt-5.6-sol

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 11, 2026 3:41pm

Request Review

@benvinegar

Copy link
Copy Markdown
Member Author

@greptile-apps review

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR moves bundled VCS implementations behind the public extension contract and introduces an app-owned catalog threaded through startup, loading, reload, and watch behavior.

  • Moves Git, Jujutsu, and Sapling implementations beside bundled extensions.
  • Adds staged external-adapter repository discovery and provider-neutral project-root resolution.
  • Publishes structural source-too-large results through the extension API.
  • Updates session selectors to choose the nearest registered repository root.
  • The staged bootstrap is incomplete on reload and executes provisional extension factories twice.

Confidence Score: 3/5

The PR should not merge until external-repository reload performs staged discovery and provisional extension loading avoids executing extension factories twice.

External-only repositories can lose repository configuration and local extensions when a live session moves into them, while initial startup can duplicate unscoped extension-factory side effects during its provisional and final load passes.

Files Needing Attention: src/ui/AppHost.tsx, src/app/startup.ts

Important Files Changed

Filename Overview
src/app/startup.ts Adds two-pass external-VCS bootstrap, but the second full load executes first-pass extension factories again without cleanup.
src/ui/AppHost.tsx Threads the bundled catalog through reloads but omits the staged root/config pass needed when an external adapter recognizes the new repository.
src/app/sessionBootstrap.ts Correctly composes bundled and extension adapters into the catalog used by final loading.
src/core/config.ts Makes project-root and default-VCS resolution catalog-dependent and exposes the resolved project root.
src/core/vcs/index.ts Refactors provider-neutral adapter ordering, detection, lookup, and operation dispatch around VcsCatalog.
src/extensions/host.ts Accepts app-supplied reserved ids; production callers preserve bundled-id protection, while each invocation still runs factories in a fresh registry.
packages/session-broker-core/src/selectors.ts Adds containment-distance matching so repository subdirectories select the nearest live session root.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Resolve config with bundled VCS catalog] --> B[Load global, config, and CLI extensions]
  B --> C[Collect provisional VCS adapters]
  C --> D{External adapter changes project root?}
  D -- No --> G[Build final session catalog and review]
  D -- Yes --> E[Resolve repository config with provisional catalog]
  E --> F[Load all extensions again]
  F --> G
  H[Live-session reload] --> I[Resolve config with bundled catalog]
  I --> J[Optionally reload extensions]
  J --> G
  I -. Missing staged root pass .-> E
Loading
Prompt To Fix All With AI
### Issue 1
src/ui/AppHost.tsx:123-127
**Reload skips external root discovery**

When a live-session reload moves into a repository recognized only by an external VCS adapter, configuration is resolved with the bundled catalog and extensions are loaded without the staged root-resolution pass used at initial startup. The reload therefore omits that repository's `.hunk/config.toml` and repo-local extensions, causing incorrect preferences or a missing adapter.

### Issue 2
src/app/startup.ts:309-317
**Staged bootstrap reruns extension factories**

When an external adapter changes the discovered project root, startup loads every global, configured, and CLI extension into a fresh registry a second time without cleaning up the first execution. Extensions whose factories start watchers, open connections, register process handlers, or write state leave duplicated work or leaked resources.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat(extensions): make VCS adapters self..." | Re-trigger Greptile

Comment thread src/ui/AppHost.tsx
Comment thread src/app/startup.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant