Repository navigation
fix: keep patch-package patches applied through warm --refresh - #4068
Draft
janicduplessis wants to merge 3 commits into
Draft
janicduplessis wants to merge 3 commits into
janicduplessis wants to merge 3 commits into
Conversation
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
…s npm's effective config
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In a monorepo whose app is a nested npm package with its own lockfile, outside the root workspaces,
warm --refreshleft patch-package patches unapplied in the source checkout. The copy then carried them, unapplied, into every new worktree. The affected repo has.npmrcignore-scripts=true+legacy-peer-deps=trueat the root and inapp/, and"postinstall": "patch-package"in both places. Four things combined:warmedWorkspace, but the refresh resolves its app withmainCheckoutAppDirfromfindProjectRoot(cwd). From the worktree root, that gives the repository root, so the app's deps and pods were never refreshed. ThewarmedWorkspacefallback 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, sodepsPlanskipped it.ignore-scriptsdrops the project's own postinstall. Stim passes no script flags.npm cihonours the project's.npmrc, so patch-package never ran after a Stim install. The project sets this on purpose and documentsnpm run postinstallas a manual step after every install.npm lsnever converges. Underlegacy-peer-deps,npm ls --allexits 1 for missing peers even right after a freshnpm ci.depsPlanchecks that before the completed-install record, so every refresh ran a fullnpm 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
warmedWorkspace, falling back to the old lookup). It keeps the repository root's lockfile current too, because the copy carries the root'snode_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 runpod installinapp/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*.patchfile under it) is an install input. A custompatch-package --patch-diris not covered. It is checked both for "changed between HEAD and upstream" and in the install record. A changed patch gets a fullnpm ci, not just a patch-package re-run, because patch-package can't apply a changed patch over a file that is already patched.npm ci, ifpackage.jsondefinespostinstallandnpm config get ignore-scriptssaystruein that directory (npm's effective value: the project.npmrc, but also a user or globalignore-scripts=trueornpm_config_ignore_scripts), the refresh runsnpm run postinstallas its owndepsstep, under the same claim.npm runruns a named script even underignore-scripts, so dependencies' install scripts stay blocked. Overriding with--ignore-scripts=falsewould 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.npm lsreports right after the install. A failingnpm lsforces 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 passesnpm lsstops looping.Risk: the record gains
patchesandtreeProblemsfields. Older records still read, and older Stim reads new ones. A repo that already has apatches/dir reinstalls once after upgrading, because the old record has no patches hash.doctorshares thenpm lsprobe and will still report such a tree as stale. That is unchanged here.Test plan
New unit cases in
worktree-refresh.test.ts:depsPlancovers patches changed, patches mismatching the record, andnpm lsfailing before vs after the last install.npm ci+npm run postinstallunderignore-scripts=true, and withnpm cialone when it'sfalse. The root is skipped, and a second refresh isseed current.npm lsrejects is installed once, then skipped, and is installed again when its problems change.npm run postinstall, and a plain warm refusesSTIM_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
.npmrcignore-scripts=true+legacy-peer-deps=true. Upstream changes onlyapp/patches/dep+1.0.0.patch(v1 to v2).warm --refreshruns from the linked worktree root with an isolatedSTIM_HOMEthat hasmain/appregistered.stim 1.23.0:
This branch:
node --test test/e2e/worktree-warm.e2e.jspasses, 11/11. The full unit suite passes exceptscripts/agent-benchmark/macos-processes.test.mjs, which needssandbox-execand fails inside my sandboxed shell. It is unrelated to this diff.