cli: resolve and install transitive registry dependencies (with tests)#2
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a84322825
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const resolvedItem of installPlan) { | ||
| const result = await installItem(resolvedItem, { |
There was a problem hiding this comment.
Block example items from transitive add installs
runAdd rejects hyperframes:example only for the requested item, but this loop installs every resolved dependency without type checks. Because registryDependencies are name-only, a component/block can depend on an example and add will then write example targets (including files like index.html) into an existing project, which can overwrite user content. Add a guard that rejects example-type dependencies before installation.
Useful? React with 👍 / 👎.
| const registryEntry = entryByName.get(itemName); | ||
| if (!registryEntry) { |
There was a problem hiding this comment.
Resolve root dependency from the entry already matched
resolveItemWithDependencies first matches the requested name with entries.find(...), but fetching is done through entryByName where later duplicates overwrite earlier ones. If registry.json has duplicate names, the requested root can resolve to a different entry/type than the one that passed the existence check, making installs order-dependent. Use the matched root entry directly (or reject duplicate names up front) to keep resolution deterministic.
Useful? React with 👍 / 👎.
Motivation
add/install flow handles transitiveregistryDependenciesso components/blocks that depend on other registry items are installed in the correct order.Description
resolveItemWithDependenciesinpackages/cli/src/registry/resolver.tsto fetch manifests, walkregistryDependenciestransitively, detect cycles, and return a topologically ordered list ofRegistryItems, and adaptedresolveItemto use it.addcommand inpackages/cli/src/commands/add.tsto resolve and install transitive dependencies, build aninstalledlist in the result, and adjust snippet selection to use the final install target.packages/cli/src/registry/index.tsand added/updated tests inpackages/cli/src/registry/resolver.test.tsandpackages/cli/src/commands/add.test.tsto cover dependency ordering, missing/dependency errors, cycles, and updated install expectations.IMPROVEMENT_OPPORTUNITIES.mdat repo root with a static scan of code-level opportunities and suggested next steps.Testing
vitestunit tests inpackages/cli/src/registry/resolver.test.ts, including dependency ordering, missing-dependency, and cycle detection scenarios, and they passed.vitestunit tests inpackages/cli/src/commands/add.test.ts, which were updated to expect dependency installs and additional written files, and they passed.Codex Task