[Move] The new-site clone onto the bundled Git, partial rather than shallow (#385)

.github/instructions/code-review.instructions.md

@@ -61,7 +61,7 @@ Invariants. Breaking one is a `[fix here]` finding even when the code works on t
 
 **Child processes run on Electron's bundled Node, never the host's.** Spawns go through `process.execPath` with `ELECTRON_RUN_AS_NODE=1` in the environment (see `runNpmWithEngineRetry` and the `playground:start` handler in `src/main.js`, and `buildChildEnv`). A bare `spawn('node')` or `spawn('npm')` assumes a host toolchain that is not there. On Windows child `npm` processes find a `node` at all only because of the `PATH` shim built by `ensureNodeShimDir` — new spawns must inherit that environment rather than build their own. The one exception is the bundled Git, which is not a Node process and gets its own environment from `src/git-binary.cjs` (next invariant).
 
-**Git is the binary the app ships, never the host's.** Since #364 the app bundles Git through `dugite`, unpacked from `app.asar`. `require('dugite')` appears in exactly one file, `src/git-binary.cjs`; every Git spawn resolves the binary with its `resolveGitBinary`, takes its environment from `buildGitEnv`, its options from `SPAWN_OPTIONS` (which already sets `detached` the way section 4 asks), and starts its arguments with `BASE_ARGS`. That env drops every `GIT_*` variable the host had (dugite would otherwise honour `LOCAL_GIT_DIRECTORY` and `GIT_EXEC_PATH` and run a different Git), turns the host's system and global config off, and turns prompting off, so the host's shell or `~/.gitconfig` cannot change what the app does. A `spawn('git')` that relies on `PATH`, a hand-joined path into the dugite tree, a `GitProcess.exec` outside that file, a Git call given `buildChildEnv`'s environment, or one spawned without an explicit `cwd` is a regression. Parse only porcelain-stable output, with the flag that pins it (`--porcelain=v2`, `-z`, an explicit `--format`); parsing human-facing output is a finding however convenient. `src/git-run.cjs` is the only module that spawns the binary and `src/git-read.cjs` the only one that parses its output; a new read belongs there, with a parser test on fixture bytes, not inline at a call site. Since #384 every read outside the write flows runs on the bundled Git and returns the same shapes the `isomorphic-git` calls returned (status rows included), so a facade signature that changes with the engine is a finding; the writes and the clone, with the reads inside them, still run on `isomorphic-git` until #385, and patch and diff generation stays hand-rolled in `src/main.js` until the phase that moves it.
+**Git is the binary the app ships, never the host's.** Since #364 the app bundles Git through `dugite`, unpacked from `app.asar`. `require('dugite')` appears in exactly one file, `src/git-binary.cjs`; every Git spawn resolves the binary with its `resolveGitBinary`, takes its environment from `buildGitEnv`, its options from `SPAWN_OPTIONS` (which already sets `detached` the way section 4 asks), and starts its arguments with `BASE_ARGS`. That env drops every `GIT_*` variable the host had (dugite would otherwise honour `LOCAL_GIT_DIRECTORY` and `GIT_EXEC_PATH` and run a different Git), turns the host's system and global config off, and turns prompting off, so the host's shell or `~/.gitconfig` cannot change what the app does. A `spawn('git')` that relies on `PATH`, a hand-joined path into the dugite tree, a `GitProcess.exec` outside that file, a Git call given `buildChildEnv`'s environment, or one spawned without an explicit `cwd` is a regression. Parse only porcelain-stable output, with the flag that pins it (`--porcelain=v2`, `-z`, an explicit `--format`); parsing human-facing output is a finding however convenient. `src/git-run.cjs` is the only module that spawns the binary and `src/git-read.cjs` the only one that parses its output; a new read belongs there, with a parser test on fixture bytes, not inline at a call site. Since #384 every read outside the write flows runs on the bundled Git and returns the same shapes the `isomorphic-git` calls returned (status rows included), so a facade signature that changes with the engine is a finding; the new-site clone runs on it too (`src/git-clone.cjs`, the one place that reads Git's human-facing progress lines, because there is no porcelain for progress); the remaining writes, with the reads inside them, still run on `isomorphic-git` until #385, and patch and diff generation stays hand-rolled in `src/main.js` until the phase that moves it.
 
 **`electron-store` is the only persistence layer.** No database, no sidecar JSON. It holds the site registry and per-site metadata and is the single source of truth for "known sites". A second store, a cache file, or state parked in a module-level variable that outlives a handler is architectural drift — flag it.
 

AGENTS.md

@@ -87,7 +87,7 @@ When asked to add an existing pull request to an existing stack, preserve its co
 ## Architecture notes (non-obvious)
 
 - **Child processes run on Electron's own Node, not the system Node.** `npm install`, `npm run <script>`, and the Playground server are spawned via `process.execPath` + `ELECTRON_RUN_AS_NODE=1` — this is the mechanism behind "zero prerequisites." On Windows this requires shimming `node`/`npm`/`npx` into `PATH` so child `npm` processes can find a `node` binary at all.
-- **Git has no host dependency.** The app ships its own Git binary via `dugite` (#364), unpacked from `app.asar` and resolved only through `src/git-binary.cjs`, which also builds the environment it runs with (host system and global config off, prompting off, no `GIT_*` variable inherited from the host and nothing from a Node child). `require('dugite')` lives in that one file; `src/git-run.cjs` is the only place the binary is spawned, and `src/git-read.cjs` holds every read and its parser. Spawning a `git` found on `PATH`, or calling dugite's `GitProcess` from anywhere else, is a regression. As of #384 every read outside the write flows (status, branches, history, blobs, the patch walk) runs on the bundled binary; the writes and the clone, including the reads inside them, still run on `isomorphic-git` until #385 moves them flow by flow. Only porcelain-stable output (`--porcelain=v2`, `-z`, explicit `--format`) is ever parsed. Patch/diff generation is done by hand in `main.js`, not `git diff`: one status scan against the branch point each ticket recorded (#108), nothing staged, and `/dev/null` naming whichever side of an addition or a deletion does not exist — the app reads its own patches back when a mentor applies one, and that filename is the only thing its parser reads an add or a delete from (#85).
+- **Git has no host dependency.** The app ships its own Git binary via `dugite` (#364), unpacked from `app.asar` and resolved only through `src/git-binary.cjs`, which also builds the environment it runs with (host system and global config off, prompting off, no `GIT_*` variable inherited from the host and nothing from a Node child). `require('dugite')` lives in that one file; `src/git-run.cjs` is the only place the binary is spawned, and `src/git-read.cjs` holds every read and its parser. Spawning a `git` found on `PATH`, or calling dugite's `GitProcess` from anywhere else, is a regression. As of #384 every read outside the write flows (status, branches, history, blobs, the patch walk) runs on the bundled binary, and since #385's first flow so does the new-site clone (`src/git-clone.cjs`: partial, `--filter=blob:none`, repo config written at clone time); the remaining writes, including the reads inside them, still run on `isomorphic-git` until #385 moves them flow by flow. Only porcelain-stable output (`--porcelain=v2`, `-z`, explicit `--format`) is ever parsed. Patch/diff generation is done by hand in `main.js`, not `git diff`: one status scan against the branch point each ticket recorded (#108), nothing staged, and `/dev/null` naming whichever side of an addition or a deletion does not exist — the app reads its own patches back when a mentor applies one, and that filename is the only thing its parser reads an add or a delete from (#85).
 - **`electron-store` is the only persistence layer** — no separate DB. It holds the site registry and per-site metadata; treat it as the single source of truth for "known sites."
 - Long-running child-process output (installs, scripts, server) is streamed to the renderer via correlated IDs (`installId`/`runId`), not returned synchronously — expect async event handlers, not return values, when tracing that flow.
 

docs/guide/creating-a-site.md

@@ -21,9 +21,9 @@ Click **Create site** (or press Enter) to start. **Cancel** or Escape closes the
 
 ## What happens during setup
 
-The app clones the `wordpress-develop` repository from GitHub. Git is bundled with the app as `isomorphic-git`, a pure JavaScript implementation, so no system Git installation is involved.
+The app clones the `wordpress-develop` repository from GitHub with the Git it ships inside the app, so no system Git installation is involved. The clone carries the full history but fetches file contents on demand, so it is not much larger than a shallow one.
 
-While the clone runs, the site view shows a **Setting up new site…** card with the current phase and a terminal panel streaming progress output. The clone downloads the full repository, so expect it to take several minutes depending on your connection.
+While the clone runs, the site view shows a **Setting up new site…** card with the current phase and a terminal panel streaming progress output. Expect it to take a few minutes depending on your connection.
 
 The clone is the first step of the [initial setup checklist](./setup-wizard); the remaining steps stay locked until it finishes. Then the app carries on by itself — installing the dependencies and running the first build without waiting for you — so the only step left to click is starting the dev server.
 

docs/guide/getting-started.md

@@ -67,7 +67,7 @@ Every ticket you link gets its own branch inside the site and keeps its own work
 
 ## How it works under the hood
 
-- Git operations are handled by `isomorphic-git`, a pure JavaScript implementation of Git.
+- Git operations run on a Git binary bundled inside the app (never one installed on your machine). Some of the older flows still use `isomorphic-git`, a pure JavaScript implementation, while they are being moved over.
 - Node scripts and npm commands run on the Node.js runtime bundled with the Electron app. A small shim directory is injected into the `PATH` so subprocesses find `node`, `npm`, and `npx` without a system install.
 - The WordPress server runs on `@wp-playground/cli` from [WordPress Playground](https://wordpress.github.io/wordpress-playground/), backed by SQLite.
 - Patches are generated with the `diff` npm package.

src/git-binary.cjs

@@ -77,7 +77,18 @@ const PINNED_ENV = Object.freeze({
 	// /dev/null as "no file" for this variable, and Git for Windows maps the
 	// name itself, so the literal works on every platform.
 	GIT_CONFIG_GLOBAL: '/dev/null',
-	GIT_TERMINAL_PROMPT: '0'
+	GIT_TERMINAL_PROMPT: '0',
+	// Git's human-facing output is translated through gettext, and Git for
+	// Windows ships the translations (the macOS dugite payload has none, so
+	// nothing here or in CI would ever show it). The one place the app reads
+	// that output is the clone's progress and its `fatal:` line
+	// (git-clone.cjs); on a localised Windows the phase names come back
+	// translated, and a non-Latin locale means the contributor watches a
+	// several-minute clone with no progress at all. `C` is the untranslated
+	// locale, and it also pins the number formatting the parsers assume.
+	// LANGUAGE, which would otherwise outrank it, is ignored by gettext once
+	// the locale is C.
+	LC_ALL: 'C'
 });
 
 // Arguments every Git call site puts before its own. `credential.helper` is

src/git-clone.cjs

@@ -0,0 +1,183 @@
+'use strict';
+
+/**
+ * Creating a site: the clone of `wordpress-develop` through the bundled Git
+ * (#385, flow 1 of phase 3 of #364). The first write the binary makes, and
+ * the one with the narrowest blast radius: a repository that did not exist
+ * before.
+ *
+ * Partial, not shallow. `--filter=blob:none` brings the whole history down
+ * (commits and trees, about 57 MB for wordpress-develop) and fetches a blob
+ * only when something asks for it, which a checkout of HEAD does and nothing
+ * the app does afterwards needs. What that buys is a merge base for every
+ * pull request without a full clone (#351), and a route `isomorphic-git`
+ * never had. A shallow clone would be smaller by that 57 MB and have no
+ * history at all. Measured in the #364 spike: 5.2 s for the history alone.
+ *
+ * The repository's own config is written at clone time, so a site the binary
+ * made never depends on the CRLF view git-read.cjs synthesises for sites the
+ * old engine made: `core.autocrlf=false` keeps the tree LF on every platform,
+ * which is what wordpress-develop's blobs are and what the patch builder
+ * assumes; `core.symlinks=false` matches what the old engine wrote; and on
+ * Windows `core.longpaths=true` because the tree has paths past MAX_PATH.
+ *
+ * Progress arrives on stderr as the lines Git prints for a human, which is
+ * the one place the app reads non-porcelain output: Git has no machine
+ * format for progress, the lines have had the same shape for fifteen years,
+ * and a line that does not parse is dropped rather than shown.
+ */
+
+const path = require('path');
+const { spawnGit, GitError } = require('./git-run.cjs');
+
+/**
+ * The branch a new site checks out. `trunk` is the pristine snapshot every
+ * ticket branch is diffed against (ticket-branches.js).
+ */
+const DEFAULT_BRANCH = 'trunk';
+
+/**
+ * @param {Object} root0
+ * @param {string} root0.url
+ * @param {string} root0.dir        Must not exist yet, or be empty.
+ * @param {string} [root0.branch]
+ * @param {string} [root0.platform]
+ * @return {string[]}
+ */
+function cloneArgs({ url, dir, branch = DEFAULT_BRANCH, platform = process.platform }) {
+	return [
+		'clone',
+		'--filter=blob:none',
+		'--single-branch',
+		'--branch', branch,
+		'--progress',
+		'--config', 'core.autocrlf=false',
+		'--config', 'core.symlinks=false',
+		...(platform === 'win32' ? ['--config', 'core.longpaths=true'] : []),
+		'--',
+		url,
+		dir
+	];
+}
+
+// `Receiving objects:  42% (1234/5678), 12.00 MiB | 3.00 MiB/s`, with or
+// without the `remote: ` prefix the server-side phases carry.
+const PROGRESS_LINE = /^(?:remote: )?([A-Za-z][A-Za-z ]*?):\s+(\d+)% \((\d+)\/(\d+)\)/;
+
+/**
+ * One progress event per phase line, from a chunk that may hold several
+ * lines and may end mid-line. Git separates updates to the same phase with
+ * `\r` and phases with `\n`; both end a segment here.
+ *
+ * @param {string} text
+ * @return {Array<{phase: string, percent: number, loaded: number, total: number}>}
+ */
+function parseProgressLines(text) {
+	const events = [];
+	for (const segment of text.split(/[\r\n]/)) {
+		const match = PROGRESS_LINE.exec(segment);
+		if (!match) continue;
+		events.push({
+			phase: match[1].toLowerCase(),
+			percent: Number(match[2]),
+			loaded: Number(match[3]),
+			total: Number(match[4])
+		});
+	}
+	return events;
+}
+
+/**
+ * Feeds chunks in, emits complete progress lines out, and holds back the
+ * tail that has not ended yet so a percentage split across two chunks is not
+ * reported twice, or wrongly.
+ *
+ * @param {Function} onEvent
+ * @return {{push: Function, flush: Function}}
+ */
+function createProgressReader(onEvent) {
+	let pending = '';
+	const emit = (text) => {
+		for (const event of parseProgressLines(text)) onEvent(event);
+	};
+	return {
+		push(chunk) {
+			pending += chunk;
+			const cut = Math.max(pending.lastIndexOf('\r'), pending.lastIndexOf('\n'));
+			if (cut === -1) return;
+			emit(pending.slice(0, cut + 1));
+			pending = pending.slice(cut + 1);
+		},
+		flush() {
+			if (pending) emit(pending);
+			pending = '';
+		}
+	};
+}
+
+/**
+ * Clones `url` into `dir`. Resolves when the checkout is complete; rejects
+ * with a GitError carrying Git's stderr when it is not, in which case `dir`
+ * holds whatever Git left and the caller decides what to do with it.
+ *
+ * @param {Object}   root0
+ * @param {string}   root0.url
+ * @param {string}   root0.dir
+ * @param {string}   [root0.branch]
+ * @param {Function} [root0.onProgress] `{ phase, percent, loaded, total }`
+ * @param {Function} [root0.onChild]    Handed the ChildProcess, so a quit can
+ *                                      kill it (killChildTree).
+ * @param {string}   [root0.platform]
+ * @param {Function} [root0.spawn]      Injection point for tests.
+ * @return {Promise<{dir: string}>}
+ */
+function cloneSite({ url, dir, branch = DEFAULT_BRANCH, onProgress = null, onChild = null, platform = process.platform, spawn } = {}) {
+	return new Promise((resolve, reject) => {
+		const args = cloneArgs({ url, dir, branch, platform });
+		// The parent is the working directory: `dir` may not exist yet, and a
+		// clone is the one command whose target is an argument, not the cwd.
+		const cwd = path.dirname(dir);
+		let child;
+		try {
+			child = spawnGit(args, { cwd, ...(spawn ? { spawn } : {}) });
+		} catch (error) {
+			reject(error);
+			return;
+		}
+		if (onChild) onChild(child);
+
+		const reader = createProgressReader((event) => { if (onProgress) onProgress(event); });
+		const stderr = [];
+		let settled = false;
+		child.stdout.on('data', () => {});
+		child.stderr.on('data', (chunk) => {
+			const text = chunk.toString('utf8');
+			stderr.push(text);
+			reader.push(text);
+		});
+		child.on('error', (error) => {
+			if (settled) return;
+			settled = true;
+			reject(new GitError(`git clone could not start: ${error.message}`, { code: error.code, signal: null, stderr: '', args, cwd }));
+		});
+		child.on('close', (status, signal) => {
+			if (settled) return;
+			settled = true;
+			reader.flush();
+			if (status === 0) {
+				resolve({ dir });
+				return;
+			}
+			const text = stderr.join('');
+			// Git's reason is its `fatal:` (or `error:`) line, which is not always
+			// the last one: "Please make sure you have the correct access rights
+			// and the repository exists." follows it. Fall back to the last line
+			// that is not a progress update.
+			const lines = text.split(/[\r\n]/).filter((line) => line.trim() && !PROGRESS_LINE.test(line));
+			const reason = lines.filter((line) => /^(fatal|error):/.test(line)).pop() || lines.pop() || (signal ? `killed by ${signal}` : 'no output');
+			reject(new GitError(`git clone failed (${status === null ? signal : status}): ${reason}`, { code: status, signal, stderr: text, args, cwd }));
+		});
+	});
+}
+
+module.exports = { DEFAULT_BRANCH, cloneArgs, parseProgressLines, createProgressReader, cloneSite };