Skip to content

refactor: extract shared VCS infrastructure - #1005

Open
benvinegar wants to merge 1 commit into
mainfrom
refactor/package-first-phase-2-vcs
Open

refactor: extract shared VCS infrastructure#1005
benvinegar wants to merge 1 commit into
mainfrom
refactor/package-first-phase-2-vcs

Conversation

@benvinegar

@benvinegar benvinegar commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

  • add the private @hunk/vcs workspace under packages/hunk-vcs
  • extract provider-neutral process, diff-target, path, source, and large-file infrastructure
  • preserve the canonical import-free hunkdiff/extension contract
  • migrate bundled providers to explicit shared helper subpaths and enforce package boundaries

Validation

  • typecheck, lint, Knip, dependency boundaries
  • full unit suite and focused provider tests
  • npm build, pack validation, NodeNext and Bundler consumers

This is PR 1 of 4 in Phase 2. It intentionally does not extract a provider or change activation behavior.

This PR description was generated by Pi using GPT-5.6 Sol

@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Sep 6, 2026 5:08am UTC

Request Review

@benvinegar benvinegar changed the title refactor/package first phase 2 vcs refactor: extract shared VCS infrastructure Sep 6, 2026
@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts provider-neutral VCS infrastructure into a new private @hunk/vcs workspace and rewires Git, Jujutsu, Sapling, and compatibility modules to consume explicit package subpaths.

  • Adds package, source-boundary, dependency-cruiser, Knip, TypeScript, Bun lock, and Nix lock configuration.
  • Moves subprocess, path, source-text, and diff-target helpers into the new dependency-bottom package.
  • Preserves selected legacy Hunk imports through compatibility re-exports.
  • Adds package-boundary and isolated type-resolution coverage.

Confidence Score: 4/5

The PR appears safe to merge after correcting the non-blocking Node compatibility claim in the private VCS package.

The extraction and provider rewiring are internally consistent, but the new package advertises Node support despite exporting Bun-dependent runtime implementations and only testing type resolution.

Files Needing Attention: packages/hunk-vcs/package.json, packages/hunk-vcs/src/async-process.ts, packages/hunk-vcs/src/source.ts

Important Files Changed

Filename Overview
packages/hunk-vcs/package.json Defines the private helper package and explicit source exports, but overstates Node runtime compatibility.
packages/hunk-vcs/src/source.ts Relocates bounded file and stream readers plus subprocess cleanup into the new package.
packages/hunk-vcs/src/async-process.ts Moves the existing abortable Bun subprocess implementation without behavioral changes.
packages/hunk/src/extensions/default/vcs/git/commands.ts Rewires Git command helpers to consume explicit @hunk/vcs subpaths.
scripts/hunk-vcs-package.test.ts Checks package boundaries and TypeScript resolution, but does not validate the declared Node runtime.
scripts/source-boundaries.test.ts Extends architectural checks to keep the new package dependency-bottom and provider imports constrained.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    ExtensionAPI["hunkdiff/extension contract"]
    VCS["@hunk/vcs helper leaves"]
    Git["Bundled Git provider"]
    JJ["Bundled Jujutsu provider"]
    Sapling["Bundled Sapling provider"]
    Compat["packages/hunk/src/lib compatibility exports"]
    Core["Hunk core callers"]

    Git --> ExtensionAPI
    JJ --> ExtensionAPI
    Sapling --> ExtensionAPI
    Git --> VCS
    JJ --> VCS
    Sapling --> VCS
    Compat --> VCS
    Core --> Compat
Loading
Prompt To Fix All With AI
### Issue 1
packages/hunk-vcs/package.json:36
**Node Support Is Misstated**

This package declares support for Node 22+, but its exported runtime modules use Bun-only APIs such as `Bun.spawn` and `Bun.file`, and the exports point directly to TypeScript source. A Node consumer can therefore meet the stated engine requirement but fail when loading or invoking these helpers. Please remove the Node engine claim or provide and test a Node-compatible implementation.

---

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

Reviews (1): Last reviewed commit: "refactor: extract shared VCS infrastruct..." | Re-trigger Greptile

},
"engines": {
"bun": ">=1.3.14",
"node": ">=22"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Node Support Is Misstated

This package declares support for Node 22+, but its exported runtime modules use Bun-only APIs such as Bun.spawn and Bun.file, and the exports point directly to TypeScript source. A Node consumer can therefore meet the stated engine requirement but fail when loading or invoking these helpers. Please remove the Node engine claim or provide and test a Node-compatible implementation.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/hunk-vcs/package.json
Line: 36

Comment:
**Node Support Is Misstated**

This package declares support for Node 22+, but its exported runtime modules use Bun-only APIs such as `Bun.spawn` and `Bun.file`, and the exports point directly to TypeScript source. A Node consumer can therefore meet the stated engine requirement but fail when loading or invoking these helpers. Please remove the Node engine claim or provide and test a Node-compatible implementation.

---

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

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