Skip to content

feat(git-native): 'native' git impl#52

Draft
zdunecki wants to merge 2 commits intoScelarOrg:mainfrom
livesession:feat/git-native
Draft

feat(git-native): 'native' git impl#52
zdunecki wants to merge 2 commits intoScelarOrg:mainfrom
livesession:feat/git-native

Conversation

@zdunecki
Copy link
Copy Markdown
Contributor

This PR makes git integration more native because current implementation is API-heavy. The goal is to make a Nodepod to operate on git more natively - thanks to that we don't need a Github API at all.

Current solution is experimental AND does not cover all cases - currently it's focused on cloning repos. For example if clone happens via native then it do that thanks to isomorphic-git which is a great git implementation for node/browser.

Constraints:

  • Big repos cloning speed is not sufficient but during tests I saw that current impl also does not work best.

Notes:

  • I've added e2e tests to showcase it works.

I think it's still experimental feature but looks like a big opportunity for the future. Nevertheless tests shows it works and locally I'm at least satisfied to make this PR to look for.

@R1ck404
Copy link
Copy Markdown
Contributor

R1ck404 commented Apr 28, 2026

Hey, thanks for putting this together. The idea is something I want and you clearly put work in.

Before this can move forward I'd want it restructured to fit the rest of the repo a bit better. Few things if you want to take another pass at it:

The git-shim/ folder splits one operation across six files (clone-api, clone-native, fs-adapter, index, types, worker-rpc). Everything else in src/packages/ is a single flat file, look at installer.ts or registry-client.ts for the shape. I'd collapse the whole thing into one src/packages/git-client.ts with a single GitClient class that has a clone method dispatching internally on mode. The strategy split makes sense once there's a pull and push, but for one operation it's premature.

The new "git-clone-request" message goes around the typed WorkerToMainMessage union in src/threading/worker-protocol.ts. The default: this.emit(msg.type, msg) change in process-handle is an escape hatch I'd rather not have. Every other worker to main message has a typed interface in that file and a typed case in the switch, please add yours the same way.

For the dispatch itself, ProcessManager already has the pattern you need. Look at how http-request works: there's _httpCallbacks: Map<number, ...>, _nextHttpRequestId, and a dispatchHttpRequest() that returns a promise. Reuse that shape with a dispatchGitRequest. Then you don't need the separate GitShim.attach() hooking into processManager.on('spawn').

The buffer.ts and crypto.ts changes I'd want pulled out. Buffer is foundational, making set() clamp instead of throw to silence one library's bug will hide real overflows elsewhere. Same with crypto, mixHash got swapped for real SHA via sha.js but kept the same name, which silently changes the synchronous digest() semantics for every caller. If isomorphic-git really needs proper SHA, do that as its own PR with a clear scope.

The Playwright setup is a worthwhile addition on its own. Worth its own PR so this one doesn't pull a whole new test framework along with it.

A few small cleanups while you're in there: drop the as any casts (the existing packages have zero of them), no empty catch {} blocks, trailing newlines on new files. And there's a real bug: progress.done() fires twice in the api path, once inside CloneApi.execute and again in the wrapper after .then(). Also the require.resolve("@scelar/nodepod") plus /dist/ to /src/ regex in the vite plugin is fragile, will only work in some install layouts.

Totally fine if you'd rather not do the rewrite, I can take it from here. Either way thanks for the push to actually think about native git, hadn't really considered it before this.

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.

2 participants