fix(using-git-worktrees): lockfile-aware Step 3 (pnpm/yarn/bun + uv)#1562
Open
iKon85 wants to merge 1 commit into
Open
fix(using-git-worktrees): lockfile-aware Step 3 (pnpm/yarn/bun + uv)#1562iKon85 wants to merge 1 commit into
iKon85 wants to merge 1 commit into
Conversation
Step 3 was hardcoded to `npm install` whenever `package.json` exists. In pnpm/yarn workspaces this is wrong: `npm install` - ignores `workspace:*` protocol declarations, - won't create `node_modules/<pkg>` symlinks for workspace packages, - can produce a `package-lock.json` alongside the existing `pnpm-lock.yaml` / `yarn.lock`. The downstream effect is silent: tools that resolve imports through `node_modules` (LSP, tsserver, ts-node, type checkers) see a partial module graph. In a multi-package repo, references in one package may resolve while the other package's resolution quietly breaks. Empirical repro from a real pnpm workspace (testreporter): a freshly created worktree without setup ran LSP findReferences on a shared type and returned 54 refs across 19 files. After `pnpm install` in the worktree, the same query returned 94 refs across 29 files — the missing 40 refs were all in the workspace package importing via the package name (`@scope/shared`) rather than a tsconfig path alias. Fix: - Detect the package manager from the lockfile and run the matching install: `pnpm-lock.yaml` → pnpm, `yarn.lock` → yarn, `bun.lockb` → bun, `package-lock.json` → npm, fall back to npm if only `package.json` is present. - Same lockfile-aware logic for Python: prefer `uv.lock` (uv sync), then `poetry.lock`, then `pyproject.toml` (poetry install), then `requirements.txt` (pip). No regression for npm-only projects (still hits `npm install` in the fallback). No new tooling required by the skill itself.
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.
What problem are you trying to solve?
using-git-worktreesStep 3 ("Project Setup") is hardcoded tonpm installwhenever apackage.jsonis present:```bash
Node.js
if [ -f package.json ]; then npm install; fi
```
In pnpm or yarn workspaces this is wrong.
npm install:workspace:*protocol that pnpm/yarn lockfiles declare,node_modules/<pkg>symlinks for sibling workspace packages,package-lock.jsonnext to an existingpnpm-lock.yaml/yarn.lock, leaving the tree in a hybrid state.Failure mode is silent. Tools that resolve through
node_modules(LSP, tsserver, ts-node, type-aware refactors) get a partial module graph and return partial results. AfindReferenceson a workspace-shared type comes back with fewer matches than reality. Nothing logs an error; the agent gets a wrong answer and proceeds.Empirical repro (real pnpm workspace, two packages with one importing the other via package name, the other via tsconfig path alias):
pnpm installThe 40 missing refs were all in the package importing via the workspace package name (
@scope/shared). With nonode_modules/@scope/sharedsymlink, tsserver couldn't load that project into the reference search at all.npm installfrom the current Step 3 would have produced the same broken state plus a straypackage-lock.json.Initial prompt that led to this change: "Arbeite an #271 — using-git-worktrees-Skill pnpm/yarn-aware machen. #270 hat empirisch gezeigt, dass das in pnpm-Workspaces zu LSP-Bruch führt." (Tracking in the downstream project: iKon85/Testreporter#270, iKon85/Testreporter#271.)
What does this PR change?
Detects the package manager from the lockfile and runs the matching install. Falls back to npm when only
package.jsonis present. Same lockfile-aware shape applied to the Python block (uv → poetry → poetry → pip), because the original ran both pip and poetry whenrequirements.txtandpyproject.tomlcoexisted.Is this change appropriate for the core library?
Yes. The skill already commits to auto-detecting and running setup; this PR fixes the detection so it matches what the project actually uses. The change is not project-specific, team-specific, or tool-specific — it makes a generic skill correct in the common case of pnpm/yarn/bun workspaces, which the skill currently misbehaves on. No new third-party integration.
What alternatives did you consider?
command -v pnpm/which yarn. Rejected: a developer can havepnpminstalled globally and still use npm in a given repo; the lockfile is the authoritative signal for what this project uses.Does this PR contain multiple unrelated changes?
The Node.js block is the primary change. The Python block applies the same lockfile-aware pattern and fixes an adjacent bug (the original runs
pip installandpoetry installif bothrequirements.txtandpyproject.tomlexist). If reviewers prefer to land Node.js alone, I'll happily split out Python into a follow-up — let me know.Existing PRs
Environment tested
Evaluation
pnpm install, which we already know produces the correctnode_modulesstate in that repo. The fix is mechanical (lockfile selector); the risk surface is narrow.Rigor
superpowers:writing-skillsand completed adversarial pressure testingHonest disclosure:
superpowers:writing-skillswas invoked, but the formal RED-GREEN-REFACTOR-with-subagent-pressure-tests workflow was deliberately skipped after user agreement, on the reasoning that #270 is the RED, the fix is a 5-line shell selector with no behavioral surface beyond "pick the right binary," and a synthetic pressure scenario wouldn't surface anything the production repro didn't already show. Re-test on request.Human review