fix(upgrade): detectBunLink fails because bun resolves symlinks in argv[1]#704
Closed
MrAladdin wants to merge 1 commit into
Closed
fix(upgrade): detectBunLink fails because bun resolves symlinks in argv[1]#704MrAladdin wants to merge 1 commit into
MrAladdin wants to merge 1 commit into
Conversation
…gv[1]
bun resolves the entire symlink chain before setting process.argv[1],
so lstatSync(argv1).isSymbolicLink() always returns false for bun-link
installs, short-circuiting the git-config walk that would correctly
identify the repo. Remove the symlink gate — argv[1] is already the
real path inside the checkout, which is what the walk needs.
Also: return { repoRoot } so the upgrade path can auto-execute
git pull + bun install via execFileSync (no shell injection surface).
Fixes garrytan#368, supersedes incomplete v0.28.5 fix for garrytan#656.
5 tasks
Owner
|
Closing — your fix landed in master via the v0.30.3 fix-wave PR #776 (merged at Thank you for the contribution — credit is preserved in the v0.30.3 CHANGELOG entry. 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
gbrain upgradereturnsunknownfor bun-link installs — the README's recommended standalone install path (git clone && bun install && bun link).Root cause:
detectBunLink()(added in v0.28.5, closes #656) gates onlstatSync(process.argv[1]).isSymbolicLink(). But bun resolves the entire symlink chain before settingprocess.argv[1], soargv[1]is already the real path (e.g./path/to/gbrain/src/cli.ts), never a symlink. The check always returnsfalse, short-circuiting the git-config walk that would have correctly identified the repo.Verified with a minimal reproduction:
Related: #368 (open — the broader bun-link upgrade story), #656 (closed by v0.28.5 but fix incomplete), #538 (open PR with alternative approach, stale against master).
Fix
Minimal change to the existing
detectBunLink():Remove the broken symlink gate —
lstatSync/isSymbolicLinkcheck deleted.argv[1]is already the real path inside the checkout, which is exactly what the git-config walk needs.Return
{ repoRoot }instead of'bun-link'— the upgrade path now knows the checkout location and can auto-execute the upgrade.Auto-upgrade instead of printing manual instructions — the
bun-linkcase inrunUpgrade()now runs:Uses
execFileSyncwith array args (no shell) per the codebase pattern indry-fix.ts:172, addressing the shell-injection concern raised in fix(upgrade): detect README "Standalone CLI" installs as 'source' #538's review. Falls back to a clear manual command on failure.--ff-onlyprevents surprise merge commits — if the user has local commits, git gives a clear error rather than silently creating a merge.Before
After
Tests
test/upgrade.test.ts— 3 new tests (12 → 15 total, all pass):detectBunLink does not gate on isSymbolicLink— regression guard; extracts the function body and asserts noisSymbolicLink/lstatSyncdetectBunLink returns repoRoot, not a string literal— pins the return shape contractbun-link upgrade uses execFileSync for shell-injection safety— pinsexecFileSyncwith array args (notexecSyncwith template strings)Typecheck passes (
bun run typecheck).Out of scope
'source'approach — this fix keeps the existing'bun-link'type)check-update.ts(already returns'gbrain upgrade'forbun-link)Need help on this PR? Tag
@codesmithwith what you need.