Skip to content

fix(install): deduplicate binary aliases - #963

Merged
benvinegar merged 2 commits into
mainfrom
fix/install-conflict-aliases
Sep 1, 2026
Merged

fix(install): deduplicate binary aliases#963
benvinegar merged 2 commits into
mainfrom
fix/install-conflict-aliases

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • deduplicate competing Hunk paths by canonical executable identity rather than display path
  • calculate PATH shadowing with canonical identities so symlinked directories report the correct precedence
  • cover duplicate foreign aliases, managed-directory aliases, and the scripted force environment variable

Follow-up to #960.

Validation

  • bun test scripts/install-sh.test.ts — 16 passed
  • bun run typecheck
  • sh -n install.sh
  • shellcheck -s sh install.sh
  • bunx oxfmt --check scripts/install-sh.test.ts
  • git diff --check
  • independent subagent review: no blocker or major findings

This PR description was generated by Pi using GPT-5.6 Sol

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hunk-web Ready Ready Preview Sep 1, 2026 12:07am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR deduplicates competing Hunk aliases using canonical executable identities and uses those identities to calculate PATH shadowing through symlinked directories.

  • Adds canonical-identity tracking alongside display paths in the installer.
  • Resolves candidate and managed executable identities while determining PATH precedence.
  • Adds regression coverage for foreign aliases, managed-directory aliases, and the scripted conflict override.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking issue where alias-first PATH ordering can degrade package-manager-specific remediation.

Canonical deduplication and shadowing behavior are covered by focused tests, but discarding later recognizable physical paths can cause the installer to classify a known installation as generic.

Files Needing Attention: install.sh

Important Files Changed

Filename Overview
install.sh Canonical identities now drive alias deduplication and PATH precedence, but retaining only the first display alias can degrade package-manager classification and remediation.
scripts/install-sh.test.ts Adds focused regression cases for canonical directory aliases, duplicate foreign aliases, and the force environment variable.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  PATH[PATH entries and managed-tool scans] --> Candidate[Candidate executable path]
  Candidate --> Canonical[Resolve canonical executable identity]
  Canonical --> Seen{Identity already seen?}
  Seen -->|Yes| Skip[Suppress duplicate alias]
  Seen -->|No| Store[Store identity and first display path]
  Store --> Shadow[Compare canonical PATH positions]
  Store --> Classify[Classify retained display path]
  Classify --> Remediation[Render conflict remediation]
Loading
Prompt To Fix All With AI
### Issue 1
install.sh:184-190
**Alias retention degrades remediation**

When a symlinked alias appears before a recognizable Homebrew, npm, nvm, or mise path for the same executable, canonical deduplication retains only the alias for channel classification, producing generic package-manager removal guidance instead of the available channel-specific remediation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(install): deduplicate binary aliases" | Re-trigger Greptile

Comment thread install.sh
Comment on lines +184 to 190
if [ -n "$hunk_candidate_identities" ] && printf '%s\n' "$hunk_candidate_identities" | grep -Fqx "$candidate_identity"; then
return 0
fi
hunk_candidate_identities="${hunk_candidate_identities}${hunk_candidate_identities:+
}${candidate_identity}"
hunk_candidates="${hunk_candidates}${hunk_candidates:+
}${candidate}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Alias retention degrades remediation

When a symlinked alias appears before a recognizable Homebrew, npm, nvm, or mise path for the same executable, canonical deduplication retains only the alias for channel classification, producing generic package-manager removal guidance instead of the available channel-specific remediation.

Prompt To Fix With AI
This is a comment left during a code review.
Path: install.sh
Line: 184-190

Comment:
**Alias retention degrades remediation**

When a symlinked alias appears before a recognizable Homebrew, npm, nvm, or mise path for the same executable, canonical deduplication retains only the alias for channel classification, producing generic package-manager removal guidance instead of the available channel-specific remediation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in ff89c3e. Deduplication still uses canonical executable identity, but diagnostics now retain every discovered alias and prefer a manager-shaped alias with the same identity for channel classification and removal guidance. The regression test models npm’s real nvm bin/hunk symlink into lib/node_modules/hunkdiff/bin/hunk.cjs and verifies the version-specific npm command is retained.

Responded by Pi using openai/gpt-5.6-sol.

This comment was generated by Pi using GPT-5.6 Sol

@benvinegar
benvinegar merged commit 06410e3 into main Sep 1, 2026
14 checks passed
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