Blocker — both PATH merges append, so the static guesses win over the user's real order.

extraPathDirs concatenates [...system, ...userBinDirs, ...npmPrefix] here, and appendToEnvPath only ever pushes, so /opt/homebrew/bin and /usr/local/bin always precede the user-level dirs — and the login shell's authoritative answer is appended even later (line 117). The result is that we resolve a different binary than the user's terminal does, which is the opposite of what this PR sets out to do.

Concretely: someone once ran npm i -g picgo under a Homebrew node (leaving a stale /opt/homebrew/bin/picgo whose node is gone), then reinstalled with pnpm add -g picgo into ~/Library/pnpm. Their login shell puts ~/Library/pnpm first, so picgo --version works in a terminal. Here, PATH ends up …:/opt/homebrew/bin:…:~/Library/pnpm:…, command -v picgo returns the stale shim, detection shows a green check, and every upload fails with an opaque node error.

The shell-imported dirs are the most authoritative thing we have and should be prepended; the static guesses are the least authoritative and should come last.

Regression — /usr/local/lib/node_modules/.bin is gone.

Both of the candidate lists this PR deletes (in cmd.ts and uploader.ts) checked exactly this path via fs.pathExistsSync. It is absent from SYSTEM_BIN_DIRS / extraPathDirs.

For a user who installed with sudo npm i -g on a layout that materialises /usr/local/lib/node_modules/.bin/picgo, and whose login shell does not carry that dir: on develop, mt::cmd::exists('picgo') returned true and resolvePicgoBinary() returned that absolute path. After this PR neither commandExists.sync (dir isn't on PATH) nor the new dir scan covers it, so the panel says "not installed" and uploads reject with PicGo command not found in PATH.

Coverage shrinking is worth avoiding in a PR whose goal is to widen it.

Bug (reproduced) — execFile has a timeout but no maxBuffer, so a chatty rc file silently discards the entire PATH import.

Verified by reproduction: with a startup file emitting ~1.2 MB, execFile errors with stdout maxBuffer length exceeded, stdout is truncated at exactly 1048576 bytes, and because echo MARKER; printenv PATH runs after everything the rc files print, the marker is gone. The backwards scan then yields marker === -1 and readLoginShellPath resolves [] — with no log line. A long motd, a fortune/figlet banner, or a verbose tool bootstrap is enough.

Two fixes needed here:

1. Pass an explicit maxBuffer (and log when it is hit).
2. The if (error && !stdout) guard on line 94 lets a partially truncated run be parsed, which can append a half-written directory to PATH. It should bail on any error unless the output is provably complete (see the closing-sentinel comment below).

Bug — the marker parser takes the first non-empty line after the marker, with no closing sentinel.

Line 91 requests an interactive shell (-i), which means per-command hooks run: powerlevel10k's instant prompt, iTerm2/VS Code shell integration, a user precmd/preexec that prints anything. That output lands between echo __MARKTEXT_PATH__ and printenv PATH, so lines.slice(marker + 1).find(non-empty) returns the hook's line, split(':').filter(isAbsolute) yields [] (or one nonsense dir), and the import silently does nothing.

The sting is that this fails for exactly the users whose shell setup is elaborate enough to need the fix in the first place.

Printing a second marker after printenv PATH and taking what's strictly between the two makes this unambiguous, and also gives line 94 a reliable completeness check.

Bug — failures are memoised, so one bad launch disables command discovery for the whole session.

shellEnvPath ??= importShellEnvPath() caches the promise regardless of outcome. If the 5s SHELL_TIMEOUT is hit because the machine was busy at launch (cold start, Spotlight indexing, slow nvm/conda init), or the rc file was momentarily broken, or the maxBuffer case above fired, the resolved-empty promise is returned forever.

The preferences panel re-polls mt::cmd::exists every 30s (uploader/index.vue, scheduleNextDetection) and can never recover: the user stares at "your system does not have picgo installed" for the entire session, when a restart would fix it.

Resetting shellEnvPath = null on a failed or empty result lets a later caller retry.

Cleanup — this fallback scan is unreachable in the running app.

setupEnvironment calls patchEnvPath() (env.ts:72), which appends exactly extraPathDirs(process.platform, process.env) to process.env.PATH. commandExists.sync on line 33 then runs command -v <name> in a child that inherits that PATH, and command -v accepts precisely the executable regular files that the new isRunnable accepts. So once line 33 has returned nothing, lines 41-44 cannot return non-null — in the main process they never fire. They only do work in specs that skip patchEnvPath (picgo-discovery.spec.ts, resolve-command.spec.ts).

The cost is two mechanisms owning the same directory list, plus a stat of 12-13 dirs on every negative answer — on a handler the preferences panel polls three times per 30s cycle.

Worth picking one owner: either patchEnvPath owns the dirs and resolveCommandPath is purely a PATH lookup, or resolveCommandPath owns the search and patchEnvPath stops mutating PATH.

Design — this eagerly runs the user's interactive login shell on every launch, before --safe is even parsed.

Both consumers already await ensureShellEnvPath() (cmd.ts:8, uploader.ts:155), so the eager start buys nothing for correctness — only pre-warming. What it costs is executing $SHELL -ilc with the user's full interactive+login startup path on every single launch: ssh-agent/gpg-agent spawns, direnv hooks evaluated against cwd (/ for a Dock launch), history-file writes, ~/.bash_logout on exit, plus the orphaned background job noted in envPath.ts.

It also runs before safeMode = !!args['--safe'] is computed on line 86 — so --safe, whose whole purpose is to avoid user configuration, cannot suppress it.

Dropping the eager call, or gating it on safeMode and on the uploader/pandoc features actually being reachable, keeps the fix without the unconditional side effect.

Test hazard — this can execute the developer's real picgo and upload the fixture to their live image host.

The comment just above states the intent: "The stand-in must be the only one reachable." HOME=tmpDir removes the ~/... dirs, but the SYSTEM_BIN_DIRS half of extraPathDirs (/opt/homebrew/bin, /usr/local/bin) is HOME-independent and is still scanned by resolveCommandPath's fallback.

So whenever the fake login shell doesn't deliver — a runner where /bin/sh can't exec the temp script, a sandbox that blocks child spawns, either of the maxBuffer/marker failures above — commandExists.sync('picgo') returns false and the loop finds the developer's Homebrew picgo instead. The "uploads with the very same picgo the check found" test then execFiles a real picgo with a real PNG against whatever remote they have configured.

Pointing the spec at a name no machine can plausibly have — as the e2e spec already does with mt-e2e-uploader-5518 — removes the hazard outright.

Test gap — the app under test inherits the runner's full PATH, so the reported scenario is never exercised.

helpers.ts:76-79 copies every entry of process.env into the child env and then Object.assigns options.env over it. The app therefore receives the developer's login-shell PATH (~/Library/pnpm, nvm shims and all) with only SHELL replaced. The comment here — "PATH is left as the launcher gave it" — is true only in the trivial sense that the launcher in CI is a terminal.

The regression #5518 actually describes is a reduced PATH containing little more than the /etc/paths entries, and that is not reproduced. A future change that re-breaks discovery under a minimal PATH while still working under a rich one passes this suite.

Passing an explicit minimal PATH alongside SHELL in options.env would make the test match its own comment.

Test bug — this writes the literal string 'undefined' when PATH was unset.

originalPath is string | undefined, and plain assignment coerces undefined to 'undefined' rather than unsetting the variable. A run where PATH was absent leaves every later spec in that worker with PATH='undefined', failing all command lookups.

The adjacent if (originalHome === undefined) delete process.env.HOME is the intended pattern — PATH should use it too. Same line appears at picgo-discovery.spec.ts:78.