Skip to content

fix: Native CLI setup avoids duplicate PATH entries - #1621

Merged
hatayama merged 1 commit into
v3-betafrom
fix/hatayama/normalize-native-cli-path-entry
Jul 8, 2026
Merged

hatayama merged 1 commit into
v3-betafrom
fix/hatayama/normalize-native-cli-path-entry

Conversation

@hatayama

@hatayama hatayama commented Jul 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Prevent native CLI setup from leaving duplicate current-process PATH entries when an existing entry has a trailing separator.
  • Keep unrelated PATH entry text and ordering unchanged.

User Impact

  • Repeated native CLI setup no longer leaves equivalent install directories in the Unity process PATH solely because one form ends with a separator.
  • CLI resolution behavior and the persisted User PATH remain unchanged.

Root Cause

  • BuildPathWithInstallDirectory compared raw PATH entry text while removal and ownership checks compared normalized path identities.
  • Equivalent paths with different trailing separators were therefore treated as different entries during setup.

Changes

  • Normalize the install directory and each non-empty existing PATH entry before equality comparison.
  • Continue appending the original text for unrelated entries.
  • Add a Windows trailing-separator regression test.

TDD Verification

  • Red: the new regression test retained both the canonical install directory and the trailing-separator duplicate.
  • Green: the regression test passes with one canonical install entry.
  • Unity compile: 0 errors, 0 warnings
  • NativeCliInstallerTests: 33 passed
  • Related detector, PATH setup, and application service fixtures: 31 passed
  • No IPC or protocol changes

Review in cubic

Compare existing PATH entries by normalized path identity before prepending the native CLI directory. This prevents trailing separators from preserving a duplicate current-process PATH entry while retaining the original text of unrelated entries.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5115a378-5fbb-4e86-a077-7782d141dcf8

📥 Commits

Reviewing files that changed from the base of the PR and between cbd1674 and f7759de.

📒 Files selected for processing (2)
  • Assets/Tests/Editor/NativeCliInstallerTests.cs
  • Packages/src/Editor/Infrastructure/CLI/NativeCliInstallPathResolver.cs

📝 Walkthrough

Walkthrough

The PATH comparison logic in NativeCliInstallPathResolver.BuildPathWithInstallDirectory was updated to normalize both the install directory and each path segment before comparing them, preventing duplicate entries with trailing separators. A corresponding test was added.

Changes

PATH deduplication fix

Layer / File(s) Summary
Normalize path comparison and add regression test
Packages/src/Editor/Infrastructure/CLI/NativeCliInstallPathResolver.cs, Assets/Tests/Editor/NativeCliInstallerTests.cs
BuildPathWithInstallDirectory normalizes the install directory and each non-whitespace path segment before comparing them, and a new test verifies duplicate entries with trailing separators are removed from the resulting PATH string.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing duplicate PATH entries during native CLI setup.
Description check ✅ Passed The description is directly related to the changeset and accurately explains the PATH de-duplication fix and regression test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hatayama/normalize-native-cli-path-entry

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

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Re-trigger cubic

@hatayama
hatayama merged commit 345c027 into v3-beta Jul 8, 2026
10 checks passed
@hatayama
hatayama deleted the fix/hatayama/normalize-native-cli-path-entry branch July 8, 2026 14:40
@github-actions github-actions Bot mentioned this pull request Jul 11, 2026
RyanXie123 pushed a commit to RyanXie123/unity-cli-loop that referenced this pull request Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant