[Update] Run the trunk update and the discards on the bundled Git (#385)

[juanmaguitar]

Why

src/trunk-update.js held the last writes on isomorphic-git outside patch apply: the fetch-and-reset behind Update to latest trunk and the two discards. Its reads already ran on the bundled Git (#402); the writes did not, which is why fetchDepth had to guess per site whether a fetch would re-shallow a partial clone, why the fetch suite carried a hand-rolled smart-HTTP server (isomorphic-git has no file://), and why the update fetched a URL fixed in the app instead of the checkout's own remote (#359). With #405 below this, no site the update can reach is shallow, so all three go. Stacked on #405.

What changes

• src/git-run.cjs: streamGit(args, { cwd, onProgress, onStderr, onChild, spawn }), the one runner for the commands that report progress. cloneSite and checkoutBranch are wrappers over it now (the same 35-line spawn-and-read block used to live in both); the fetch is its third caller. Rejects with the last fatal: line, not the first line of a screen of progress.
• src/git-write.cjs: fetchBranch(dir, remote, branch) (fetch --progress --no-tags -- <remote> <branch>, then FETCH_HEAD; no --depth, no --filter: the site's own promisor config keeps the fetch partial; only https, http and file allowed as transports, since the URL is the site's own config), unstagePaths (reset -q by NUL pathspec on stdin, literal), cleanUntracked (clean -fd: ignored and excluded files stay, a nested repository stays).
• src/git-read.cjs: isAncestor (merge-base --is-ancestor, exit 0/1) and remoteUrl (one config read).
• src/trunk-update.js: updateToLatestTrunk({ dir, onLog, onChild }): url is gone; it fetches origin trunk, compares the lockfile blob between the two tips as before, unstages the stale index entries, update-ref refs/heads/trunk <new> <trunk's own oid> (a second writer fails loudly instead of being overwritten), forced checkout. Git's own fetch lines reach the terminal as printed. discardChanges / discardToBase: forced checkout of the current branch plus clean -fd, the rewind still guarded by ancestry. fetchDepth deleted. ensureAutocrlf / createCrlfCompatibleFs stay exported for patch-apply.js and sites:add until the patch flow moves; nothing here calls them.
• src/main.js: noOriginBlock, the shape of legacySiteBlock and midSwitchBlock: a site with no origin remote is refused with the app's sentence and code: 'no-origin' before the ticket is parked. The update, and both discards, pass onChild: trackGitChild(sitePath) so a quit mid-fetch or mid-checkout finds the child. WORDPRESS_GIT_URL keeps its one consumer, the clone. The legacySiteBlock comment no longer claims ensureAutocrlf writes core.autocrlf (it is an in-memory view for isomorphic-git; it never wrote the repository).
• Tests: trunk-update-fetch.integration.test.cjs loses the loopback server and clones its site with cloneSite from a file:// origin: the same eight cases, the two wire-level deepen assertions replaced by "no .git/shallow, promisor intact, full history walkable", plus fetch-from-the-checkout's-origin (E2E: cover the flows the git-native refactor will move #359), both children handed out, and a held ref lock tagged stage: 'checkout' with the tree untouched. Argv tests for the three primitives and streamGit, integration tests for unstagePaths / cleanUntracked / fetchBranch / isAncestor, two more discard cases (ignored and excluded files survive; an unknown base does not rewind). New journey trunk-update.spec.js: makeSite({ origin: true }) clones the site beside itself, the test moves that origin ahead, More → Update to latest trunk, and asserts the file, the branch, the substrate, no shallow boundary, and the store's new snapshot with no incomplete marker.
• Docs: AGENTS.md and the review standard say that nothing under src/ requires isomorphic-git any more (it stays in package.json and the fixtures until Phase 4: isomorphic-git leaves, and the issues parked on this decision are re-asked #386); TESTING.md no longer says a suite starts a server; docs/guide/trunk-updates.md says the update fetches from the site's own origin and what a site without one gets.

Diff size. About 1,370 changed lines in two commits (the move, then the self-review answers), over the ~800 the process asks to declare: 460 of them are the fetch suite (the server goes, the file:// fixture comes), and the runner extraction is net negative in git-clone.cjs / git-write.cjs. Splitting the primitives from their only caller would have left a PR with no behaviour to test.

How to test this

Platforms: macOS and Windows.

Starting state: the Buildkite artifact for the current head installed; one site created by this build (or by #403's), on trunk, built.

1. More → Update to latest trunk on a site whose trunk has moved. Expected: Git's own remote: Counting objects… / Receiving objects… lines stream in the terminal, then Resetting to latest trunk (…), then Now on trunk as of …; install skipped unless package-lock.json moved; the green summary afterwards. In the site folder: git rev-parse --is-shallow-repository prints false, git config remote.origin.promisor prints true, git log --oneline | wc -l is the full history.
2. Run it again. Expected: Already up to date. and the confirmation, no install, no build.
3. Link a ticket, edit a file, More → Update to latest trunk → Discard them. Expected: the edit is gone, node_modules intact, the site returns to the ticket branch after the update.
4. Discard your changes on a dirty ticket branch with an untracked file and a staged one (git add from a terminal). Expected: both gone, ignored files intact, still on the ticket branch, parked work intact.
5. Quit the app while Receiving objects is on screen. Expected: no git fetch left in ps / Task Manager; reopening the site shows no half state (the fetch touched no file).
6. Windows: steps 1 and 4; git status --porcelain clean afterwards, git config --local core.autocrlf still false.
7. Optional: a site added from disk whose origin is a fork. Expected: the update fetches from the fork (From <fork url> in the terminal). A site added from disk with no origin (git remote remove origin): the terminal says it has no origin remote and nothing is parked or fetched.

What must not have happened: a .git/shallow in any site; a fetch of tags (git tag prints nothing); node_modules or build/ removed by a discard; a site left on trunk with the ticket still linked after a failed update (the existing log line says where the work went).

Risks and limitations

• A site with no origin is refused before anything moves, with the sentence and how to add one; an origin with no trunk, or one reached over ssh (a transport the fetch does not allow, since it would spawn a host ssh), fails at the fetch with Git's own reason, tagged stage: 'fetch', nothing moved. The app never makes such a site; one adopted from disk can be. Before this PR the same site would have fetched the fixed URL and silently updated to WordPress/wordpress-develop whatever its origin was.
• clean -fd is what the old discard did by hand, minus one thing: the old code deleted only paths statusMatrix listed, which honoured .gitignore and .git/info/exclude the same way. A nested repository inside the site (which nothing in the app creates) is left alone; the old code would have deleted its files.
• Windows CRLF: the discards and the update prefix windowsArgs like every other worktree write, for sites adopted from a host Git; a site the app cloned carries core.autocrlf=false and the prefix is a no-op.
• ensureAutocrlf / createCrlfCompatibleFs and their six tests stay until the patch flow moves; so does isomorphic-git in the fixtures of trunk-update.integration.test.cjs and the journeys helper (Phase 4: isomorphic-git leaves, and the issues parked on this decision are re-asked #386).
• Review: see the Review outcome section.

Related

Part of #364 and #385; closes the #359 requirement that the update read the checkout's remote. Stacked on #405 (merge that first). Next: patch flows, then #386.

---

Design decisions and alternatives considered

• update-ref + forced checkout, not reset --hard or merge --ff-only. reset --hard has no --progress and would hide the tag point the caller relies on (worktreeReset flips exactly when files start being overwritten); merge --ff-only refuses a trunk that has local commits, which the app never makes, so the refusal would only ever be noise. The forced ref write matches what isomorphic-git did, with expected (trunk's own value) added so a concurrent writer is a loud failure.
• reset by pathspec, not rm --cached. rm --cached refuses a path whose staged content matches neither HEAD nor the worktree; the stale entries patch generation leaves are exactly that shape.
• Raw stderr to the terminal for the fetch, parsed events for the checkout. Git's fetch lines are already what a terminal should show and carry their own \r; the checkout's are mapped to the same phase loaded/total line as before so the update's log reads the way it did.
• streamGit extracted now. The fetch would have been the third copy of the spawn-and-read block; two was already one too many, and the third is where a divergence between them (one flushing the reader, one not) becomes a bug nobody can see in a diff.
• No fetchDepth replacement. The decision behind [Add] Refuse writes on sites the old engine made, and say so on the card (#385) #405 (no old-engine sites are written) is what makes the fetch unconditional; keeping a depth branch for a case the gate excludes would be a second copy of that rule.
• A working clone as the journey's origin, not a bare one. The test commits into it with the bundled Git; a bare repository would need a second working clone to do that.

Review outcome (required — see AGENTS.md)

/self-review against the parent branch, judgement pass run with fresh context (the diff and the review standard only). npm run lint clean, unit suite green on both Node runtimes before and after (1161, then 1164), journeys 16/16. 6 [fix here] · 4 [follow-up]; the six and two of the four are in the second commit.

Applied

• Tests / cross-platform 🔴: the discard fixture never pinned core.autocrlf, and the discards now run through the same forced checkout as a switch, which on Windows would have written CRLF into six byte-for-byte assertions. Pinned like the two sibling fixtures; the CRLF-view tests ask for the unset shape explicitly.
• Architecture 🟡: a site adopted from disk with no origin used to update silently from the fixed URL and would now have failed with Git's raw fatal:. noOriginBlock in the handler refuses before the ticket is parked, with the app's sentence and code: 'no-origin'; a read that fails leaves the fetch to speak. Test in ipc-wiring.
• Docs 🟡: the rewritten sentences in AGENTS.md and the standard said patch apply was "the last write on isomorphic-git". It writes hunks with fs by hand, and after the first commit nothing under src/ requires isomorphic-git at all. Both say so now, and patch-apply.js's header no longer claims the app never shells out to Git.
• Docs 🔵: isLegacySite's comment justified "never shallow" with the deleted fetchDepth; trunk-update.js's header miscounted ensureAutocrlf's callers.
• Architecture 🔵: the update-ref guard compared against HEAD's oid, not trunk's; on a detached HEAD or a stale registry it would have failed at stage: 'checkout' with nothing moved. It compares trunk's own value now, with a test that updates from a detached HEAD.
• Security 🔵 (follow-up, taken): the fetch URL comes from the site's .git/config, so ext:: would run a command. fetchBranch pins protocol.allow=never and allows https, http and file; ssh stays off because it would spawn a host ssh.
• Tests 🔵 (follow-up, taken): the claim behind dropping --filter (the promisor config keeps the fetch partial) now has an assertion: the blob nothing checked out is still on the server after the update.

Follow-up, not here

• Dependencies 🟡: isomorphic-git is a production dependency nothing in src/ imports, and ensureAutocrlf is a no-op at its three call sites. Both go with Phase 4: isomorphic-git leaves, and the issues parked on this decision are re-asked #386, which also retires the fixtures.
• Cross-platform 🔵: clean -fd exits 1 when one untracked file cannot be removed (a file held open on Windows), where the old per-file loop carried on. Exposure is node_modules and build/ free (both ignored); left strict and stated here.

Rules the standard does not cover (left for a later edit of the instructions file): when a function or constant is deleted, grep for its name in comments elsewhere (three findings were prose two files away that the deletion made false); a design claim a comment states as the reason for a choice needs a test that fails when it stops being true.

Checked and sound: the forced checkout plus clean -fd reproduces the old discard (staged-new files, untracked, ignored), with .git/info/exclude entries now surviving where they did not; stage / worktreeReset at every exit; FETCH_HEAD correct for one explicit refspec; windowsArgs on checkout, reset, clean and not on fetch; streamGit a faithful extraction with git-progress.cjs still its one consumer; trackGitChild safe across the fetch then checkout sequence; no re-shallow or unshallow reachable.

Screenshots: none; the terminal now shows Git's own fetch lines where it showed fetch 1234/5678 before. One will be added from the Buildkite artifact.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J6koA9mvjSExVaKJp4nvSx

[juanmaguitar]

@coderabbitai review

[coderabbitai]

Warning

Review limit reached

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a1ca7628-4bb6-4c15-a7e9-10220dd9e08e

📥 Commits

Reviewing files that changed from the base of the PR and between 96bd712 and cb40851.

📒 Files selected for processing (21)

• .github/instructions/code-review.instructions.md
• AGENTS.md
• TESTING.md
• docs/guide/trunk-updates.md
• src/git-clone.cjs
• src/git-read.cjs
• src/git-run.cjs
• src/git-write.cjs
• src/main.js
• src/patch-apply.js
• src/trunk-update.js
• tests/e2e/helpers/git-site.cjs
• tests/e2e/journeys/trunk-update.spec.js
• tests/unit/git-read.integration.test.cjs
• tests/unit/git-read.test.cjs
• tests/unit/git-run.test.cjs
• tests/unit/git-write.integration.test.cjs
• tests/unit/git-write.test.cjs
• tests/unit/ipc-wiring.test.cjs
• tests/unit/trunk-update-fetch.integration.test.cjs
• tests/unit/trunk-update.integration.test.cjs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[juanmaguitar]

Windows walkthrough, 2026-09-09, Windows 11 VM with no Git installed, Buildkite artifact of #411 (a52e684, same tree as the current stack heads after the chain rebase).

"Update to latest trunk" on a fresh partial clone: "Already up to date with trunk", status --porcelain empty, core.autocrlf still false, rev-parse trunk unchanged. Trunk had not moved since the clone, so the fetch had nothing to bring; the fetch itself and the checkout ran without error.