Say which git-fi runs, and how to replace a token - #12
Merged
Merged
Conversation
chris-peterson
force-pushed
the
leftover-gem-binstubs
branch
2 times, most recently
from
August 13, 2026 20:07
662078b to
4136d91
Compare
chris-peterson
force-pushed
the
leftover-gem-binstubs
branch
from
September 6, 2026 15:43
4136d91 to
49ec692
Compare
chris-peterson
force-pushed
the
leftover-gem-binstubs
branch
3 times, most recently
from
September 15, 2026 19:58
77aa58b to
2fbb5a1
Compare
chris-peterson
force-pushed
the
leftover-gem-binstubs
branch
4 times, most recently
from
September 16, 2026 02:06
3988989 to
05ab929
Compare
One user migrating off the Ruby gem hit two dead ends in a row, and neither left him anywhere to go. He removed the gem's directory and gemspec by hand, which left the launcher RubyGems had written into Ruby's bin directory — earlier on his PATH than npm's prefix. `git fi` kept reaching that dead launcher while the npm install sat behind it, complete and unreachable. Nothing could say so: the copy that would notice is the copy not being run. What did work was `npx git-fi`, so INSTALL-01 makes `--version` compare the `git-fi` that PATH resolves against the copy answering, and name both when they differ. It reports on stderr, so stdout stays a bare version string, and only for `--version`: a second installation is not a fault, and a line on every run would be noise to whoever has a reason for it. Past that, his two-year-old GITLAB_ACCESS_TOKEN had expired, and the 401 printed GitLab's JSON, named the branch the request happened to carry, and offered one remedy — turn CI status off. He went looking for a doc on where tokens come from. git-fi already knew: AUTH-10 built the prefilled form for `--auth=login`. AUTH-13 puts that link on the 401 too, with the source that supplied the rejected token, and notes that a stored token outranks the export it replaces. None of this reproduces on macOS, and CI could not see it either: the matrix already ran windows-latest, but the suite drives the compiled binary out of the checkout, so nothing installed globally or reached git-fi by the name a shell resolves. A throwaway probe on a real runner supplied the design, and is replaced here by the guard it earned — a Windows job that installs the packed tarball, checks `git fi` reaches it, then plants a stale launcher ahead of it and checks the report names it. The job asserts that `git fi` stops answering before it asserts what the report says: checking only that git-fi named the planted file would pass against a resolver searching the wrong names, which is how the PATHEXT search survived its first review. A further step plants the shims git cannot run and requires silence. A second job builds a gem shaped like the one git-fi replaced and installs it the way its README said to, rather than imitating what it leaves: a launcher this repo writes can only ever carry the shape someone guessed at, which is how both of the above survived. Which launcher counts as in the way was measured on that runner rather than reasoned about, because the two tools a reader would check with both disagree with git. `git fi` resolves a subcommand to `git-fi.exe`, else the extensionless `git-fi`, and to nothing else — a stale `.cmd`, `.bat` or `.ps1` ahead of npm's prefix leaves it running the npm install as before. PowerShell runs the `.ps1`, so `Get-Command git-fi` names that one; `where.exe` lists the `.cmd`. Searching PATHEXT would report a file that is in nobody's way, and would misread npm's own prefix, which holds `git-fi`, `git-fi.cmd` and `git-fi.ps1` at once. Which file counts is the other half, and it is a different question per platform. `gem install --no-wrappers`, which the gem's own README told users to run, links its three executables into Ruby's bin directory rather than copying them — so removing the gem's files by hand leaves those links dangling. Windows git stops at one and dies on it, where POSIX git skips it and tries the next entry on PATH. So the Windows test is the directory entry and the POSIX test is `X_OK`, which settles both the dangling link and a file missing its execute bit in one call. Asking whether the path exists, as this first did, followed the link and so was blind to precisely the state being reported. That gap is why the user who reported this got nowhere: PowerShell cannot execute an extensionless file, so `Get-Command` skipped past the leftover Ruby launcher and named npm's `.ps1` as the winner while `git fi` was running the Ruby one. Two more findings from the probe are load-bearing. Identity is the launcher's target resolving to the copy answering, not a package-name match — two npm prefixes from an nvm or fnm switch each hold a launcher naming this same package, and matching the name would suppress exactly the shadow the notice exists for. And the check is not in the installer where it was first proposed: npm has hidden postinstall output since v7, and on npm 11 install scripts are blocked by default, so it would not run at all. Identifying which git-fi is running is only worth doing if silence is reliable, so the quiet paths carry the tests: an unresolvable entry point says nothing, and `GIT_FI_NO_HINTS` is the only switch that suppresses a notice the user just asked for — neither `$CI` nor a pipe does, unlike every other hint. `npx git-fi --version` is the way out this offers, since the copy that would notice is the copy not being run, and it had to be made to work. npx prepends its own cache to PATH and links the copy it runs into it, so the walk found git-fi's own launcher first, matched it against the copy answering, and reported nothing shadowing. The walk therefore skips the `node_modules/.bin` directories npm puts on PATH for the life of one command: a shell has none of them, and which launcher a shell reaches is the whole question. Skipping them alone answered worse than the silence did. Under npx the copy answering is a throwaway unpacked a moment ago, so the user's own global install is a different copy, and it came back named as the thing in the way under a heading telling them to delete it. The comparison the notice rests on has no useful answer there, so npx does not make it: it names the launcher PATH resolves and stops. That is what npx was run to ask, it holds whether or not anything is wrong, and it cannot point at the wrong file. A quality pass over the result consolidated what the two features had each grown their own copy of. Both tables composed the Branch column from the same four steps in the same order; both `--json` writers built and wrote their own envelope, and only one waited for the bytes to reach the OS, which is the difference between a whole document and one truncated at the pipe buffer. The git floor was a version string spelled out in two files; it is `engines.git` in package.json now, which npm records but enforces only for `node`, so both of git-fi's entry points read the field and do the enforcing. `localBranchName` was introduced as the one way to strip an `origin/` prefix and then applied only to new code; it now has every call site, and lives with `resolveBranchName` in a module that imports nothing, so the JSON and rendering layers can normalize a name without pulling in the module that spawns subprocesses. That pass also introduced a crash and a review caught it. Reading the default branch off `git symbolic-ref` and treating the answer as a ref that exists is wrong twice: the symref outlives a pruned target, and `basename` truncates a branch with a `/` in it. Either way `%(ahead-behind:)` is fatal rather than empty, so a repo whose default branch had been renamed could not list its branches at all. The name is resolved before anything treats it as one, and both states have tests. Two smaller ones from the same review. The notice's remedy named npm's prefix, where on POSIX the launcher sits in its `bin` subdirectory, so following it literally adds a directory holding no `git-fi`. And giving each SPEC section its prefix as a heading dropped `Branch List Storage` without replacing it, leaving `STORAGE-01`..`04` under Merge Process and two links to them pointing at anchors that no longer existed.
chris-peterson
force-pushed
the
leftover-gem-binstubs
branch
from
September 16, 2026 02:16
05ab929 to
e225882
Compare
chris-peterson
marked this pull request as ready for review
September 16, 2026 02:32
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.
Context
git-fi shipped as a Ruby gem until 0.9.x and now ships on npm. A user migrating across deleted the gem's files by hand, which left RubyGems' launcher in Ruby's
bin— earlier onPATHthan npm's prefix.git fikept reaching that dead launcher while the npm install sat behind it, complete and unreachable, and nothing could say so: the copy that would notice is the copy not being run. Separately, an expiredGITLAB_ACCESS_TOKENreturned a 401 whose only offered remedy was turning CI status off.A requirement for each (
INSTALL-01,AUTH-13), plus the Windows CI that made the first designable.Review guide
Tier 1 — the new behavior
src/which.ts—shadowingLauncheris the idea: resolvegit-fionPATH, compare it to the copy running, return the winner only when they differ.isSameInstallis worth a second look.warnIfShadowed— thenpxbranch, which is what makes the feature reachable whengit fiis shadowed. See Approach.src/index.ts— the only call site.--version, nothing else, on stderr so stdout stays a bare version string.src/gitlab.ts—rejectedTokenMessage.AUTH-10already built the prefilled token form for--auth=login; this puts it on the 401, sharingREQUIRED_SCOPEso the prose and the URL can't disagree.src/style.ts—hintsOptedOut. This notice wants the explicit switch without the ambient suppressions, since it answers a question just asked.Tier 2 — the CI that proves it
.github/workflows/ci.yml— two Windows jobs. One plants a leftover launcher;windows-leftover-geminstalls a gem shaped like git-fi's predecessor with--no-wrappers, then deletes its files by hand. Both checkgit fihas actually stopped working before asserting what the notice says.Tier 3 — a correctness fix the cleanup pass introduced
src/git.ts—git symbolic-refexits 0 on a pruned target, andbasenametruncates a default branch containing/. Either way%(ahead-behind:)is fatal, so a repo whose default branch was renamed couldn't list branches at all. Now resolved before use, with tests for both states.Tier 4 — consolidation, spec, docs
src/branches.ts—localBranchName/resolveBranchNamein a module importing nothing, so the rendering and--jsonlayers can strip anorigin/prefix without pulling in the one that spawns subprocesses. Every call site uses it now.src/gitlab.tsandsrc/json.ts— both branch tables built the Branch column the same four ways, and both--jsonwriters built their own envelope while only one waited for the bytes to reach the OS. OnebranchLabel, onewriteJson.package.json— the git floor was spelled out in two files; it'sengines.gitnow, read bypreflightChecksand the postinstall, since npm records the field but enforces onlynode. A test pins the range's shape: anything but>=x.y.zmakesordinalreturnNaNand silently stops the floor applying.SPEC.mdINSTALL-01andAUTH-13;STORAGEregains the heading the prefix rename dropped, which had left two links pointing at anchors that no longer existed.docs/quickstart.md— migration callout, instructions only.STATUS.md— ledger 132 → 134, all Covered.Approach & trade-offs
Which launcher is in the way was measured, because both tools you'd check with disagree with git.
git firesolves togit-fi.exe, else the extensionlessgit-fi, and nothing else — a stale.cmd,.bator.ps1ahead of npm's prefix leaves it working. But PowerShell runs the.ps1andwhere.exelists the.cmd, solauncherNamessearchingPATHEXTnamed files in nobody's way. That is also why the reporter got nowhere: PowerShell can't execute an extensionless file, soGet-Commandskipped the leftover Ruby launcher entirely and named npm's.ps1, whilegit fiwas running the Ruby one.Whether a launcher is reachable is a different question per platform.
gem install --no-wrapperslinks its executables into Ruby'sbin, so removing the gem's files leaves dangling links. Windows git stops at one and dies; POSIX git skips it for the nextPATHentry. So the test is the directory entry on Windows andX_OKon POSIX, which also covers a missing execute bit. Asking whether the path exists follows the link, and so was blind to exactly the reported state.Under
npxthe notice states a fact rather than a verdict. npx is the route out of a shadowed install, and it didn't work: npx prepends its own cache toPATH, so the walk matched git-fi's own launcher and reported nothing. The walk now skipsnode_modules/.bindirectories, which a shell never has. That alone was worse than silence — the copy answering is a throwaway, so the user's real install reads as a different copy and gets named as the culprit. So npx names the launcherPATHresolves and stops.Install identity is the launcher's target resolving to the copy answering, not a package-name match. Two npm prefixes (an nvm/fnm switch) each hold a launcher naming this package, so a name match would suppress the notice in the case it exists for.
The check is not in the installer, where it was first proposed. On npm 11 install scripts are blocked by default, so a global install skips
postinstallentirely unless the package is inallowScripts— observed on a user's machine this week. That also stopsPRE-06's install-time floor andCOMPLETE-07's completion files running at all;PRE-02still enforces the floor at first use. Both need their own fix, outside this branch.Testing
226 green on both platforms — 29 tests for
INSTALL-01, 8 forAUTH-13, 2 for the default-branch resolution, 2 for the git floor.Naming order is asserted from any host by passing the platform in, since which names to look for is a pure string decision. Reachability is not: the platform argument picks the rule, but
X_OKis answered by the host's filesystem, so those cases are gated to where the rule is real.Validation
Get-Command git-fireturnedC:\tools\ruby33\bin\git-fi; deleting it madegit firesolve to npm's copy.AUTH-13now puts in the error.git fiwhich it ran:.exeand bare only,.cmd/.bat/.ps1never.windows-leftover-gemconfirms--no-wrapperssymlinks three executables into Ruby'sbin, and that deleting the gem's files leavesgit fidying on a dangling link.npx