Skip to content

fix: keep patch-package patches applied through warm --refresh - #4068

Draft
janicduplessis wants to merge 3 commits into
mainfrom
@janic/warm-refresh-nested-installs
Draft

janicduplessis wants to merge 3 commits into
mainfrom
@janic/warm-refresh-nested-installs

Conversation

@janicduplessis

@janicduplessis janicduplessis commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Description

In a monorepo whose app is a nested npm package with its own lockfile, outside the root workspaces, warm --refresh left patch-package patches unapplied in the source checkout. The copy then carried them, unapplied, into every new worktree. The affected repo has .npmrc ignore-scripts=true + legacy-peer-deps=true at the root and in app/, and "postinstall": "patch-package" in both places. Four things combined:

  • Wrong package from the root. Warm registers and prepares the app it finds through warmedWorkspace, but the refresh resolves its app with mainCheckoutAppDir from findProjectRoot(cwd). From the worktree root, that gives the repository root, so the app's deps and pods were never refreshed. The warmedWorkspace fallback came later, in feat(status): report warming and ready phases so a workspace shows while it is set up #1629. The refresh (feat: warm --refresh updates the main checkout under the ownership claim #690) predates it.
  • patches/ is invisible. A commit that changes only a patch does not touch the lockfile, so depsPlan skipped it.
  • ignore-scripts drops the project's own postinstall. Stim passes no script flags. npm ci honours the project's .npmrc, so patch-package never ran after a Stim install. The project sets this on purpose and documents npm run postinstall as a manual step after every install.
  • npm ls never converges. Under legacy-peer-deps, npm ls --all exits 1 for missing peers even right after a fresh npm ci. depsPlan checks that before the completed-install record, so every refresh ran a full npm ci. In the affected repo, 37 of 37 refreshes did this, taking 30 s to 6 min each. Each run also wiped whatever patches were applied.

Fixes #4067

Solution

  • The refresh uses the same app as warm (warmedWorkspace, falling back to the old lookup). It keeps the repository root's lockfile current too, because the copy carries the root's node_modules. All roots are planned before any installs, so one failed install doesn't leave another root's record marked unfinished. Pods follow the app, so a root-cwd refresh can now run pod install in app/ios. Run from a non-app package with exactly one registered app, the refresh now installs that app plus the root, not the package it was run from.
  • patches/ (every *.patch file under it) is an install input. A custom patch-package --patch-dir is not covered. It is checked both for "changed between HEAD and upstream" and in the install record. A changed patch gets a full npm ci, not just a patch-package re-run, because patch-package can't apply a changed patch over a file that is already patched.
  • After npm ci, if package.json defines postinstall and npm config get ignore-scripts says true in that directory (npm's effective value: the project .npmrc, but also a user or global ignore-scripts=true or npm_config_ignore_scripts), the refresh runs npm run postinstall as its own deps step, under the same claim. npm run runs a named script even under ignore-scripts, so dependencies' install scripts stay blocked. Overriding with --ignore-scripts=false would have run every third-party script the project chose to block. This is scoped to npm. pnpm and yarn keep their patches in the lockfile.
  • The install record now stores a digest of the problems npm ls reports right after the install. A failing npm ls forces a reinstall only when its problems differ from that digest. Out-of-band damage, such as a deleted package, still triggers a reinstall, and a tree that never passes npm ls stops looping.

Risk: the record gains patches and treeProblems fields. Older records still read, and older Stim reads new ones. A repo that already has a patches/ dir reinstalls once after upgrading, because the old record has no patches hash. doctor shares the npm ls probe and will still report such a tree as stale. That is unchanged here.

Test plan

New unit cases in worktree-refresh.test.ts:

  • depsPlan covers patches changed, patches mismatching the record, and npm ls failing before vs after the last install.
  • From a monorepo root with a registered nested app, a patches-only change installs the app with npm ci + npm run postinstall under ignore-scripts=true, and with npm ci alone when it's false. The root is skipped, and a second refresh is seed current.
  • A tree npm ls rejects is installed once, then skipped, and is installed again when its problems change.
  • A failed postinstall leaves the record unfinished, the remedy names npm run postinstall, and a plain warm refuses STIM_DEPS_INCOMPLETE.

Real-tool repro with a fixture of the same shape: real npm 10, patch-package 8.0.1, a tarball dep with an unmet peer, and .npmrc ignore-scripts=true + legacy-peer-deps=true. Upstream changes only app/patches/dep+1.0.0.patch (v1 to v2). warm --refresh runs from the linked worktree root with an isolated STIM_HOME that has main/app registered.

stim 1.23.0:

deps        source main: package-lock.json unchanged -> skipped
pods        source main: no ios/ directory -> skipped
# main/app and linked/app dep: "patched v1"
# run from linked/app instead, every refresh: the installed tree does not match package-lock.json -> npm ci; dep: "original"

This branch:

deps        source main: package-lock.json unchanged -> skipped
deps        source main/app: the installed tree does not match package-lock.json -> npm ci (549ms)
deps        source main/app: ignore-scripts skipped postinstall -> npm run postinstall (250ms)
# main/app and linked/app dep: "patched v2"
# second refresh: acquired shared (seed current); main/app: package-lock.json matches the last completed install -> skipped
# deleting app/node_modules/dep: the installed tree does not match package-lock.json -> npm ci, npm run postinstall
# then a patches-only v3 upstream: main/app: patches/ does not match the last completed install -> npm ci, npm run postinstall; dep: "patched v3"

node --test test/e2e/worktree-warm.e2e.js passes, 11/11. The full unit suite passes except scripts/agent-benchmark/macos-processes.test.mjs, which needs sandbox-exec and fails inside my sandboxed shell. It is unrelated to this diff.

From a monorepo root, the refresh now installs the app warm prepares, as well
as the root lockfile, instead of only the root. A patches/ change counts as a
dependency change. After npm ci, the project's own postinstall runs as
npm run postinstall when its npm config sets ignore-scripts. A tree that npm ls
already rejected right after the last completed install no longer forces a
reinstall on every refresh.

Fixes #4067
@janicduplessis janicduplessis added the area: cli packages/stim-cli, including its README and guide label Oct 20, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli packages/stim-cli, including its README and guide

Projects

None yet

Development

Successfully merging this pull request may close these issues.

warm --refresh leaves patch-package patches unapplied in monorepos with ignore-scripts

1 participant