Skip to content

refactor: extract bundled Sapling provider - #1003

Open
benvinegar wants to merge 1 commit into
refactor/package-first-phase-2-jjfrom
refactor/package-first-phase-2-sapling
Open

refactor: extract bundled Sapling provider#1003
benvinegar wants to merge 1 commit into
refactor/package-first-phase-2-jjfrom
refactor/package-first-phase-2-sapling

Conversation

@benvinegar

@benvinegar benvinegar commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

  • move the bundled Sapling provider into private workspace packages/hunk-sapling (@hunk/sapling)
  • remove obsolete Hunk compatibility re-exports after migrating remaining callers
  • harden boundaries across all private VCS workspaces and published package checks
  • preserve provider order (jj, sl, git), Git fallback, and --no-extensions behavior

Validation

  • full unit suite: 2,084 tests
  • 150 PTY integration tests
  • typecheck, lint, Knip, dependency boundaries
  • npm and prebuilt package checks plus install smoke

This is PR 4 of 4 and completes Phase 2. Sapling executable-backed tests were unavailable locally; mocked and package-isolation coverage passed.

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 Preview Sep 6, 2026 5:12am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts the bundled Sapling implementation into a private @hunk/sapling workspace and aligns all bundled VCS providers around the public extension contract and shared @hunk/vcs leaves.

  • Adds Sapling workspace, lockfile, Nix, TypeScript, Knip, and dependency-boundary wiring.
  • Updates bundled provider composition while preserving activation order.
  • Removes host-side compatibility re-export modules and imports shared VCS helpers directly.
  • Strengthens package and architecture checks, but the new isolated provider smoke-test setup cannot resolve the unbuilt hunkdiff/extension export.

Confidence Score: 4/5

The PR is not safe to merge until the isolated provider package tests resolve the public extension entry from a built or source-compatible fixture.

All three provider package smoke tests fail in a clean checkout because the copied hunkdiff package exports hunkdiff/extension from absent generated output, making the repository’s default test suite fail.

Files Needing Attention: scripts/hunk-sapling-package.test.ts, scripts/hunk-git-package.test.ts, scripts/hunk-jj-package.test.ts

Important Files Changed

Filename Overview
packages/hunk-sapling/src/index.ts Moves the Sapling adapter into its private workspace while retaining the public extension contract and provider behavior.
packages/hunk/src/extensions/default/vcs/index.ts Replaces the relative Sapling import with @hunk/sapling without changing bundled registration order.
scripts/hunk-sapling-package.test.ts Adds package-boundary coverage, but its isolated consumer copies an unbuilt hunkdiff package whose extension export cannot resolve.
scripts/hunk-git-package.test.ts Adds hunkdiff to the isolated provider fixture using the same unbuilt-package setup that fails module resolution.
scripts/hunk-jj-package.test.ts Adds hunkdiff to the isolated provider fixture using the same unbuilt-package setup that fails module resolution.
scripts/check-pack.ts Tightens published-file validation and checks that private provider imports do not survive in the runtime bundle.
scripts/source-boundaries.test.ts Extends provider-boundary enforcement to Sapling and narrows composition-root package exceptions.
Prompt To Fix All With AI
### Issue 1
scripts/hunk-sapling-package.test.ts:70-72
**Provider smoke tests cannot load**

This copies the source `hunkdiff` workspace into the temporary consumer, but `hunkdiff/extension` resolves only to generated files under `dist/npm/extension`, which the test does not build. Importing the provider therefore fails with `Cannot find module 'hunkdiff/extension'`. Since the default suite includes `scripts/`, this breaks the test suite in a clean checkout. The Git and Jujutsu package tests use the same broken setup.

---

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 bundled Sapling provid..." | Re-trigger Greptile

Comment on lines +70 to +72
cpSync(HUNK_PACKAGE_ROOT, join(consumerRoot, "node_modules", "hunkdiff"), {
recursive: true,
});

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.

P1 Provider smoke tests cannot load

This copies the source hunkdiff workspace into the temporary consumer, but hunkdiff/extension resolves only to generated files under dist/npm/extension, which the test does not build. Importing the provider therefore fails with Cannot find module 'hunkdiff/extension'. Since the default suite includes scripts/, this breaks the test suite in a clean checkout. The Git and Jujutsu package tests use the same broken setup.

Knowledge Base Used: Distribution and release tooling

Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/hunk-sapling-package.test.ts
Line: 70-72

Comment:
**Provider smoke tests cannot load**

This copies the source `hunkdiff` workspace into the temporary consumer, but `hunkdiff/extension` resolves only to generated files under `dist/npm/extension`, which the test does not build. Importing the provider therefore fails with `Cannot find module 'hunkdiff/extension'`. Since the default suite includes `scripts/`, this breaks the test suite in a clean checkout. The Git and Jujutsu package tests use the same broken setup.

**Knowledge Base Used:** [Distribution and release tooling](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/distribution-and-release-tooling.md)

---

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

@benvinegar
benvinegar force-pushed the refactor/package-first-phase-2-sapling branch from 1286dcf to 76539e2 Compare September 6, 2026 05:12
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