Skip to content

fix(install): deduplicate binary aliases #963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benvinegar merged 2 commits into from
Sep 1, 2026
Merged

fix(install): deduplicate binary aliases #963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98cb4b4
fix(install): deduplicate binary aliases
benvinegar Aug 31, 2026
ff89c3e
fix(install): retain manager aliases
benvinegar Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/calm-install-aliases.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
34 changes: 28 additions & 6 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,33 @@ add_hunk_candidate() {
[ -x "$candidate" ] || return 0
candidate_identity="$(canonical_executable_path "$candidate")" || candidate_identity="$candidate"
[ "$candidate_identity" = "$target_identity" ] && return 0
if [ -n "$hunk_candidates" ] && printf '%s\n' "$hunk_candidates" | grep -Fqx "$candidate"; then
hunk_discovered_paths="${hunk_discovered_paths}${hunk_discovered_paths:+
}${candidate}"
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}"
Comment on lines +186 to 192

@greptile-apps greptile-apps Bot Aug 31, 2026

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!

@benvinegar benvinegar Sep 1, 2026

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

}

# Prefer a manager-shaped alias for diagnostics while canonical identity owns deduplication.
preferred_manager_path() {
preferred_candidate="$1"
preferred_identity="$(canonical_executable_path "$preferred_candidate")" || preferred_identity="$preferred_candidate"
printf '%s\n' "$hunk_discovered_paths" | while IFS= read -r discovered_candidate; do
discovered_identity="$(canonical_executable_path "$discovered_candidate")" || discovered_identity="$discovered_candidate"
[ "$discovered_identity" = "$preferred_identity" ] || continue
[ "$(competing_install_channel "$discovered_candidate")" = "another package manager" ] && continue
printf '%s\n' "$discovered_candidate"
break
done
}

# Print whether this path wins or loses against the directory this installer manages.
shadowing_direction() {
candidate_dir="$(dirname "$1")"
candidate_identity="$(canonical_executable_path "$1")" || candidate_identity="$1"
candidate_position=0
target_position=0
position=1
Expand All @@ -202,8 +219,9 @@ shadowing_direction() {
*) path_dir=$remaining_path; remaining_path=""; last_path_entry=1 ;;
esac
[ -n "$path_dir" ] || path_dir=.
[ "$path_dir" = "$candidate_dir" ] && [ "$candidate_position" -eq 0 ] && candidate_position=$position
[ "$path_dir" = "$bin_dir" ] && [ "$target_position" -eq 0 ] && target_position=$position
path_identity="$(canonical_executable_path "${path_dir%/}/hunk")" || path_identity="${path_dir%/}/hunk"
[ "$path_identity" = "$candidate_identity" ] && [ "$candidate_position" -eq 0 ] && candidate_position=$position
[ "$path_identity" = "$target_identity" ] && [ "$target_position" -eq 0 ] && target_position=$position
position=$((position + 1))
[ "${last_path_entry:-0}" = "1" ] && break
done
Expand Down Expand Up @@ -260,6 +278,8 @@ competing_install_remediation() {
# Refuse to create version skew unless the caller explicitly accepts the competing installs.
check_competing_installs() {
hunk_candidates=""
hunk_candidate_identities=""
hunk_discovered_paths=""
remaining_path=${PATH:-}
last_path_entry=0
while :; do
Expand Down Expand Up @@ -287,10 +307,12 @@ check_competing_installs() {

warn "Another Hunk installation already exists; this installer will not overwrite or remove it."
printf '%s\n' "$hunk_candidates" | while IFS= read -r candidate; do
manager_path="$(preferred_manager_path "$candidate")"
[ -n "$manager_path" ] || manager_path="$candidate"
candidate_version="$(installed_version "$candidate")"
[ -n "$candidate_version" ] || candidate_version="unknown"
warn " ${candidate} ($(competing_install_channel "$candidate"); version ${candidate_version}; $(shadowing_direction "$candidate"))"
warn " Remove with: $(competing_install_remediation "$candidate")"
warn " ${candidate} ($(competing_install_channel "$manager_path"); version ${candidate_version}; $(shadowing_direction "$candidate"))"
warn " Remove with: $(competing_install_remediation "$manager_path")"
done
fail "Remove every competing Hunk above, then try again. To knowingly keep them, rerun this installer with --force."
}
Expand Down
93 changes: 89 additions & 4 deletions scripts/install-sh.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { dirname, join, resolve } from "node:path";
import { PLATFORM_PACKAGE_MATRIX } from "./prebuilt-package-helpers";

/**
Expand Down Expand Up @@ -38,36 +38,73 @@ function writeFakeHunk(path: string, version: string) {

/** Run the installer against an already-current managed target without downloading anything. */
function runConflictCheck(
options: { force?: boolean; targetFirst?: boolean; aliasOnly?: boolean } = {},
options: {
force?: boolean;
forceEnv?: boolean;
targetFirst?: boolean;
aliasOnly?: boolean;
targetDirectoryAlias?: boolean;
duplicateForeignAlias?: boolean;
nvmAliasFirst?: boolean;
} = {},
) {
const root = mkdtempSync(join(tmpdir(), "hunk-install-conflict-"));
const home = join(root, "home");
const targetDir = join(home, ".hunk", "bin");
const foreignDir = join(root, "foreign", "bin");
const inactiveNvmDir = join(home, ".nvm", "versions", "node", "v20.0.0", "bin");
const targetAliasDir = join(root, "target-bin-alias");
const foreignAliasDir = join(root, "foreign-bin-alias");
mkdirSync(targetDir, { recursive: true });
mkdirSync(foreignDir, { recursive: true });
mkdirSync(inactiveNvmDir, { recursive: true });
writeFakeHunk(join(targetDir, "hunk"), "1.2.3");
if (options.nvmAliasFirst) {
const npmPackageBinary = join(
home,
".nvm",
"versions",
"node",
"v20.0.0",
"lib",
"node_modules",
"hunkdiff",
"bin",
"hunk.cjs",
);
mkdirSync(dirname(npmPackageBinary), { recursive: true });
writeFakeHunk(npmPackageBinary, "0.8.0");
symlinkSync("../lib/node_modules/hunkdiff/bin/hunk.cjs", join(inactiveNvmDir, "hunk"));
} else if (!options.aliasOnly) {
writeFakeHunk(join(inactiveNvmDir, "hunk"), "0.8.0");
}
if (options.aliasOnly) {
symlinkSync(join(targetDir, "hunk"), join(foreignDir, "hunk"));
} else if (options.nvmAliasFirst) {
symlinkSync(join(inactiveNvmDir, "hunk"), join(foreignDir, "hunk"));
} else {
writeFakeHunk(join(foreignDir, "hunk"), "0.9.0");
}
if (!options.aliasOnly) writeFakeHunk(join(inactiveNvmDir, "hunk"), "0.8.0");
if (options.targetDirectoryAlias) symlinkSync(targetDir, targetAliasDir, "dir");
if (options.duplicateForeignAlias) symlinkSync(foreignDir, foreignAliasDir, "dir");

try {
const systemPath = "/usr/local/bin:/usr/bin:/bin";
const pathEntries = options.targetFirst
let pathEntries = options.targetFirst
? [targetDir, foreignDir, systemPath]
: [foreignDir, targetDir, systemPath];
if (options.targetDirectoryAlias) pathEntries = [targetAliasDir, foreignDir, systemPath];
if (options.duplicateForeignAlias) {
pathEntries = [foreignDir, foreignAliasDir, targetDir, systemPath];
}
const result = Bun.spawnSync(
["sh", INSTALL_SCRIPT_PATH, ...(options.force ? ["--force"] : [])],
{
env: {
...process.env,
HOME: home,
HUNK_VERSION: "1.2.3",
HUNK_ALLOW_CONFLICTING_INSTALLS: options.forceEnv ? "1" : undefined,
PATH: pathEntries.join(":"),
},
stdin: "ignore",
Expand All @@ -83,6 +120,8 @@ function runConflictCheck(
foreign: join(foreignDir, "hunk"),
inactiveNvm: join(inactiveNvmDir, "hunk"),
inactiveNvmNpm: join(inactiveNvmDir, "npm"),
targetAlias: join(targetAliasDir, "hunk"),
foreignAlias: join(foreignAliasDir, "hunk"),
};
} finally {
rmSync(root, { recursive: true, force: true });
Expand Down Expand Up @@ -205,6 +244,41 @@ describe("hunk.dev install script", () => {
},
);

test.skipIf(process.platform === "win32")(
"uses canonical PATH identities for shadowing through a managed-directory alias",
() => {
const result = runConflictCheck({ targetDirectoryAlias: true });

expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain(
`${result.foreign} (another package manager; version 0.9.0; is shadowed by ${result.target})`,
);
},
);

test.skipIf(process.platform === "win32")(
"reports one conflict when PATH contains directory aliases to the same foreign install",
() => {
const result = runConflictCheck({ duplicateForeignAlias: true });

expect(result.exitCode).not.toBe(0);
expect(result.stderr.match(/version 0\.9\.0/g)).toHaveLength(1);
expect(result.stderr).not.toContain(result.foreignAlias);
},
);

test.skipIf(process.platform === "win32")(
"uses a canonical manager path when an unrecognized alias is discovered first",
() => {
const result = runConflictCheck({ nvmAliasFirst: true });

expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain(`${result.foreign} (npm; version 0.8.0;`);
expect(result.stderr).toContain(`'${result.inactiveNvmNpm}' uninstall -g hunkdiff`);
expect(result.stderr.match(/version 0\.8\.0/g)).toHaveLength(1);
},
);

test.skipIf(process.platform === "win32")(
"does not treat a PATH symlink to the managed binary as another install",
() => {
Expand All @@ -215,6 +289,17 @@ describe("hunk.dev install script", () => {
},
);

test.skipIf(process.platform === "win32")(
"allows the scripted force environment variable",
() => {
const result = runConflictCheck({ forceEnv: true });

expect(result.exitCode).toBe(0);
expect(result.stdout).toContain("hunk 1.2.3 is already installed.");
expect(result.stderr).toBe("");
},
);

test.skipIf(process.platform === "win32")(
"allows an explicit force flag and preserves the already-current fast path",
() => {
Expand Down
Loading