refactor(scope): use nested directories for scoped packages

[BlackHole1]

Remove the + hack that replaced / with + in scoped package names. Scoped packages like @scope/pkg-1.0.0 now use proper nested directory structure (@scope/pkg-1.0.0) instead of flattened paths (@scope+pkg-1.0.0).

Key changes:

• Remove normalizePackageName() function
• Update move() to lazily create parent directories on ENOENT
• Add comprehensive tests for scoped package handling with nested deps

BREAKING CHANGE: Existing distDir with + format directories will not be recognized. Users need to reinstall packages.

[coderabbitai]

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Fixed scoped package naming convention to use standard slash-delimited format instead of plus signs.

• New Features

  • Enhanced support for scoped packages with nested scoped dependencies during installation and listing operations.

• Tests

  • Expanded test coverage for scoped package resolution and nested dependency scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

This pull request refactors scoped package handling by removing the normalizePackageName function that previously converted slash-based package names (e.g., @scope/pkg) to plus-based formats (e.g., @scope+pkg). The changes update path constructions across install.ts, fs.ts, and related test files to use raw scoped package names directly. Three new public filesystem utility functions (exists, mkdir, walk) are exported from fs.ts, and test fixtures are added to support scoped package installation scenarios with nested dependencies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: support scope #101: Directly related path-normalization refactoring that previously introduced normalizePackageName for converting slash-delimited scopes to plus-delimited forms—this PR reverses that approach by removing the normalization and using raw scoped names.

Suggested reviewers

• crimx
• a1528zhang
• hyrious

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows the required format <type>(<scope>): <subject> with 'refactor' as type, 'scope' as scope, and a clear description of the main change.
Description check	✅ Passed	The description is directly related to the changeset, explaining the removal of the + hack, the nested directory structure changes, and listing key modifications made throughout the codebase.

_{✏️ Tip: You can configure your own custom Pre-merge Checks in the settings.}

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1eff4cf and 4dae603.

📒 Files selected for processing (16)

• src/cmd/install.test.ts
• src/cmd/install.ts
• src/cmd/list.test.ts
• src/utils/fs.test.ts
• src/utils/fs.ts
• src/utils/misc.ts
• tests/fixtures/scoped_package_install/local_storage/@foo/bar-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_install/remote_storage/@foo/bar-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_install/remote_storage/@foo/bar-2.0.0/package.oo.yaml
• tests/fixtures/scoped_package_install/remote_storage/@scope/other-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_list/local_storage/@foo/bar-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_list/local_storage_2/@scope/other-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_with_deps/entry/package.oo.yaml
• tests/fixtures/scoped_package_with_deps/local_storage/@scope/pkg-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_with_deps/local_storage/normal-dep-1.0.0/package.oo.yaml
• tests/fixtures/scoped_package_with_deps/remote_storage/@other/lib-1.0.0/package.oo.yaml

💤 Files with no reviewable changes (1)

• src/utils/misc.ts

🧰 Additional context used

🧬 Code graph analysis (3)

src/cmd/install.ts (3)

src/utils/fs.ts (1)

• exists (52-60)

src/const.ts (1)

• ooPackageName (1-1)

src/index.ts (1)

• ooPackageName (28-28)

src/cmd/list.test.ts (3)

tests/helper/fs.ts (1)

• fixture (11-13)

src/index.ts (1)

• list (15-15)

src/cmd/list.ts (1)

• list (6-20)

src/cmd/install.test.ts (4)

src/cmd/publish.ts (1)

• publish (8-33)

src/utils/fs.ts (2)

• tempDir (8-10)
• copyDir (32-43)

src/cmd/install.ts (1)

• install (49-63)

src/const.ts (1)

• ooPackageName (1-1)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Agent

🔇 Additional comments (19)

tests/fixtures/scoped_package_with_deps/local_storage/normal-dep-1.0.0/package.oo.yaml (1)

1-2: LGTM!

This test fixture correctly represents a normal (non-scoped) dependency for testing scoped package installation scenarios.

tests/fixtures/scoped_package_with_deps/entry/package.oo.yaml (1)

1-4: LGTM!

This fixture correctly defines an entry package with a scoped dependency using the proper @scope/pkg format, which aligns with the new nested directory structure.

tests/fixtures/scoped_package_with_deps/local_storage/@scope/pkg-1.0.0/package.oo.yaml (1)

1-5: LGTM!

This fixture correctly demonstrates the new nested directory structure for scoped packages, with the file located at @scope/pkg-1.0.0/package.oo.yaml and properly defining both normal and scoped dependencies.

src/cmd/install.ts (7)

7-7: LGTM!

Correctly removed the normalizePackageName import as part of the refactoring to use raw scoped package names directly.

---

133-169: LGTM!

The existence checks now correctly use raw package names (e.g., @scope/pkg-1.0.0) when constructing paths. This aligns with the new nested directory structure for scoped packages.

---

213-213: LGTM!

The existence check in installAll correctly uses raw package names for the new nested directory structure.

---

280-315: LGTM!

The target path constructions (lines 281, 302) correctly use raw package names for both already installed and newly installed dependencies, properly supporting the nested directory structure for scoped packages.

---

328-434: LGTM!

The integrityCheck function correctly uses raw package names throughout (lines 331, 400, 413) when constructing target paths and distDir paths, maintaining consistency with the new nested directory structure.

---

436-472: LGTM!

The installMissDep function correctly uses raw package names (line 459) when constructing target paths, consistent with the refactoring to support nested directories for scoped packages.

---

78-106: Parent directory creation for scoped packages is correctly handled.

The mkdir function uses fsP.mkdir(p, { recursive: true }), which automatically creates all parent directories including the @scope directory for scoped packages. The move function explicitly calls mkdir(path.dirname(dest)) before attempting the file operation, and copyDir is always invoked after mkdir(path.dirname(targetDir)) in the code flow. Both approaches ensure that nested paths like @scope/pkg-1.0.0 are properly handled.

tests/fixtures/scoped_package_with_deps/remote_storage/@other/lib-1.0.0/package.oo.yaml (1)

1-2: LGTM!

This fixture correctly represents a scoped package in the remote storage test scenario, using the proper nested directory structure @other/lib-1.0.0 and scoped package name format.

src/utils/fs.ts (2)

14-14: LGTM! Essential fix for scoped package support.

The addition of mkdir(path.dirname(dest)) ensures that parent directories (e.g., @scope) exist before attempting to rename/move packages into nested paths like @scope/pkg-1.0.0. Since mkdir uses recursive: true, it's idempotent and safe for concurrent operations.

---

97-97: LGTM! Correctly uses raw package names for scoped packages.

The removal of normalizePackageName() allows scoped packages to use their natural directory structure (@scope/pkg-1.0.0) instead of the flattened format (@scope+pkg-1.0.0). This aligns with the PR objective and standard npm conventions.

src/cmd/list.test.ts (2)

123-128: LGTM! Test expectations updated correctly.

The updated distDir paths correctly reflect the new nested directory structure for scoped packages, replacing the previous plus-delimited format with standard slash-delimited paths.

---

138-166: LGTM! Comprehensive test for nested scoped dependencies.

This new test case properly validates that the list command correctly handles scoped packages with nested scoped dependencies across multiple search directories. The test confirms all three packages are found with their correct distDir paths.

src/utils/fs.test.ts (2)

67-115: LGTM! Excellent test coverage for scoped package walking.

These tests comprehensively validate that walk() correctly:

• Resolves scoped packages using the new nested directory structure
• Populates the IDepMap with correct distDir paths using raw package names (@scope/pkg-1.0.0)
• Handles nested scoped dependencies across multiple search directories
• Properly handles missing scoped packages by setting empty distDir

---

117-138: LGTM! Proper validation of directory utilities with scoped paths.

These tests confirm that mkdir() and exists() work correctly with nested directory structures for scoped packages, ensuring both the scoped directory (@scope) and package directory (@scope/pkg-1.0.0) are created and detected properly.

src/cmd/install.test.ts (2)

674-683: LGTM! Scoped package paths updated correctly.

The publish, copyDir, and file path expectations have been consistently updated to use the new slash-delimited directory structure for scoped packages. This aligns with standard npm conventions and the PR's core objective.

Also applies to: 729-732

---

735-786: LGTM! Comprehensive test for nested scoped dependencies.

This test validates a critical scenario where:

• A scoped package (@scope/pkg) depends on another scoped package (@other/lib)
• The integrity check flow correctly resolves and installs nested scoped dependencies
• Both local and remote storage are properly searched
• The resulting distDir contains the correct package.oo.yaml files with nested directory structure

This is excellent coverage for the refactored behavior.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR refactors the package storage system to use proper nested directory structures for scoped packages (e.g., @scope/pkg-1.0.0) instead of the previous flattened format using + as a separator (e.g., @scope+pkg-1.0.0).

Key changes:

• Removed the normalizePackageName() utility function that converted / to + in package names
• Updated the move() function to create parent directories before moving files, enabling nested directory support
• Added comprehensive test coverage for scoped package handling, including nested scoped dependencies

Reviewed changes

Copilot reviewed 10 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/utils/misc.ts	Removed normalizePackageName() function that was previously used to flatten scoped package names
src/utils/fs.ts	Removed normalizePackageName import, added parent directory creation in move(), and updated walk() to use unmodified package names
src/cmd/install.ts	Removed normalizePackageName import and replaced all usages with direct package names for proper nested directory paths
src/utils/fs.test.ts	Added comprehensive tests for walk(), mkdir(), and exists() functions with scoped packages and nested dependencies
src/cmd/list.test.ts	Updated test assertions to expect nested directory paths instead of flattened paths with + separator
src/cmd/install.test.ts	Updated test assertions and added new test for scoped packages with scoped sub-dependencies via integrity check
tests/fixtures/scoped_package_with_deps/*	Added new test fixture with nested directory structure demonstrating scoped package with both normal and scoped dependencies
tests/fixtures/scoped_package_list/*	Updated fixture structure to use nested directories for scoped packages
tests/fixtures/scoped_package_install/*	Updated fixture structure to use nested directories for scoped packages

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new parent directory creation logic in the move function should be covered by tests, especially for scoped packages where nested directories are critical. Consider adding a test case that calls move() with a destination path containing nested directories (like moving to @scope/pkg-1.0.0) to verify parent directories are properly created.