Repository navigation
fix(fix): discover independent nested projects (UPT-120) - #258
Conversation
Signed-off-by: Lucas Braum <lucasbraum@gmail.com>
|
Review complete. 🟡 1 medium 💬 Inline comments (1)
🧹 Nitpicks (1) — 🟢 1 low
This change teaches
Reviewed commit: 65b75f6 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR adds discovery of nested independent projects during uptide fix, relocating the run to the matched project, with new tests and a changeset.
Key findings
- 🟡 Broken nested manifests abort dependency discovery — projects.ts:33
| const pkg = JSON.parse(readFileSync(file, 'utf8')); | ||
| return names.some((name) => | ||
| ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies'].some( | ||
| (field) => Object.hasOwn(pkg[field] ?? {}, name), | ||
| ), | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 bug · medium
Broken nested manifests abort dependency discovery
The new discovery walk probes arbitrary directories, but parses them with strict, unguarded calls: declares runs JSON.parse on every workspace member manifest with no error handling (packages/core/src/projects.ts:33), and the walk resolves nested workspaces with the strict workspacePackagesOf, which throws INVALID_WORKSPACE for a broken pnpm-workspace.yaml (packages/core/src/projects.ts:79). One malformed nested package.json (or one whose JSON is not an object, which also throws TypeError at the field access) or a broken workspace file therefore makes projectForDependency throw a raw SyntaxError, TypeError, or INVALID_WORKSPACE instead of returning a project, failing the whole fix run before any error reporting. The codebase already guards the root manifest in declaredPatterns and exports the tolerant workspacePackagesOrRoot for exactly these probe contexts, but this walk uses neither.
📋 Prompt for AI Agents
In packages/core/src/projects.ts, harden the discovery probes: (1) in declares (lines 29-40), wrap readFileSync/JSON.parse in a try/catch that treats an unreadable or invalid manifest as not declaring the dependency, and guard that the parsed value is a non-null object before the field access; (2) replace workspacePackagesOf with workspacePackagesOrRoot from './workspaces.js' for the directories probed during the walk (the declares helper, the workspacePackagesOf(child) call at line 79, and the seed call at line 85), and in isIndependentProject (lines 20-27), which is called from projectRoot (packages/core/src/fix/process.ts:127) and the ancestor loop in packages/cli/src/failures.ts:204. This prevents a malformed vendored/example package.json or a broken nested pnpm-workspace.yaml from aborting uptide fix with a raw error before the intended NO_PROJECT/NOT_REPOSITORY_ROOT diagnostics.
…oject # Conflicts: # README.md
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## uptide@0.18.0 ### Minor Changes - [#261](#261) [`93f029e`](93f029e) Thanks [@lucasbraum](https://github.com/lucasbraum)! - Sentry 10 → 11: report every `Sentry.init` whose data collection widens because `dataCollection` is unset, citing the migration guide, and give the agent the guide's exact v10 baseline. After a fix, a site counts as kept only when every category is at least that restrictive, so a partial edit such as a cookie deny list stays open in the PR instead of being reported as kept. The agent now keeps privacy settings at their most restrictive value, or leaves them as a decision, when no cited edit exists. - [#253](#253) [`e48373d`](e48373d) Thanks [@lucasbraum](https://github.com/lucasbraum)! - Find Stripe's clover billing-mode default change at subscription creation sites, across the six affected APIs. Respect request API-version pins, preserve classic billing at documented parameter paths, and report unresolved parameters or semantics as decisions. Verify quoted sources against full-page hashes at pack-build time without vendoring Stripe pages. Score the dms-saas silent billing change in UpgradeBench. ### Patch Changes - [#260](#260) [`8ea1a87`](8ea1a87) Thanks [@lucasbraum](https://github.com/lucasbraum)! - A build that fails the same way before and after the upgrade (same command, status and output; only durations and clock times may differ) no longer fails verification. Builds are now compared to the baseline the same way tests are. The PR says `⚠️ build: failing at baseline, not checked`, and the verdict reads `no new errors`, not `verification passed`. A build that passed at baseline and fails after the upgrade still fails, and so does one that fails differently. Without a pack, a run like this still does not publish, because its build cannot verify the bump. - [#259](#259) [`0db73c8`](0db73c8) Thanks [@lucasbraum](https://github.com/lucasbraum)! - Let the agent add a property the upgraded types made required to a test fixture: only to the object the compiler's TS2739, TS2740 or TS2741 error points at, and only with a literal value. Every other test input change is still rejected. Each such property is listed under "Tests changed: review" in the PR body and in report.json (file, line, property and the value the agent chose). Name a test that asserts a dependency default the upgrade changed (Sentry's `dataCollection` and `attachStacktrace`) under "Decisions for you", instead of leaving it to fail unexplained. - [#251](#251) [`773dbbe`](773dbbe) Thanks [@lucasbraum](https://github.com/lucasbraum)! - Calibrate the SDK pre-run cost estimate from recorded migrations, accounting for shared rule knowledge instead of charging a full session allowance per site. Keep the configured spending limit and per-call budget enforcement unchanged. - [#258](#258) [`3a4898f`](3a4898f) Thanks [@lucasbraum](https://github.com/lucasbraum)! - Discover independent nested projects when fixing a dependency absent from the root project. Run installation, migration and verification in the selected project's directory, preserving its lockfile and package manager. When several projects declare the dependency, name their manifests and require an explicit directory. - [#257](#257) [`7b9ed7f`](7b9ed7f) Thanks [@lucasbraum](https://github.com/lucasbraum)! - Run pinned npm through Corepack when local npm differs. When the pin is unavailable, allow local npm with a compatible v3 lockfile, record the fallback in the report, and reject frozen installs that rewrite the lockfile. Co-authored-by: uptide-release[bot] <338861287+uptide-release[bot]@users.noreply.github.com>
What and why
Fixes UPT-120. Running
uptide fix vitestat a repository root with only tooling dependencies now discovers the independentfrontend/project that declares Vitest and has its own lockfile. CLI settings and preflight, installation, migration, verification and the resulting branch all use that project.Existing root/workspace selection takes precedence. Discovery respects Git ignores and skips dependencies, generated/hidden directories and symlinks. Multiple matching projects produce an error naming their manifests and asking for an explicit
--cwd; a workspace member's stray lockfile does not make it independent.How it was verified
uptide-bench/ZM-BAD--DAG-chatat340e0b0996cf380483a320071f5b5e4d8d27340c.fix vitest --target 5.0.5 --yes --jsonand the internal rule-only switch (UPTIDE_INTERNAL=1 --no-llm, no API key). The migration completed: 178 tests in 8 files passed before and after, type checks andtsc && vite buildpassed, and onlyfrontend/package.jsonandfrontend/package-lock.jsonchanged. The original checkout was unchanged; no benchmark PR was published.pnpm build,pnpm lint,pnpm typecheck,pnpm test, andpnpm docs:links.Checklist
pnpm lint && pnpm typecheck && pnpm testpasspnpm changeset)