Skip to content

fix(cli): make executable presence detection actually probe PATH [RED-887] [ship] - #1460

Merged
sorccu merged 1 commit into
mainfrom
simo/red-887-detect-presence-missing-await
Aug 31, 2026
Merged

fix(cli): make executable presence detection actually probe PATH [RED-887] [ship]#1460
sorccu merged 1 commit into
mainfrom
simo/red-887-detect-presence-missing-await

Conversation

@sorccu

@sorccu sorccu commented Aug 31, 2026

Copy link
Copy Markdown
Member

Linear: RED-887

PathLookup.detectPresence() was missing an await on its lookupPath() call: the returned Promise was always defined, so the NotDetectedError branch was unreachable and executable-presence detection always "succeeded". Every detector's detectExecutable() delegates to it, so the executable-on-PATH fallback tier of package-manager detection unconditionally returned the highest-priority detector — pnpm — regardless of what is installed, e.g. offering pnpm install in the checkly import prompt on machines without pnpm.

This adds the await, removes the now-stale FIXME(RED-887) comment and require-await eslint-disable, and adds a direct detectPresence test pair (resolves for an executable on PATH, rejects for a nonexistent one — verified to fail with the await reverted).

Behavior change

Only the executable-fallback tier is affected, i.e. detection with no lockfile, no config file, no matching user agent, and no runtime signal:

Machine Before After
pnpm installed pnpm pnpm (no change)
pnpm absent; bun/deno/yarn/cnpm installed pnpm first installed of bun > deno > yarn > cnpm > npm
only npm installed pnpm npm
nothing installed pnpm npm (final fallback)

No working setup regresses: machines where the outcome changes were previously offered commands for a package manager that is not installed. As a side effect, an npm-workspaces monorepo with no lockfile now gets workspace detection (the wrong pnpm verdict previously found no workspace, since pnpm's lookup requires pnpm-workspace.yaml).

🤖 Generated with Claude Code

…-887]

PathLookup.detectPresence() was missing an await on its lookupPath() call,
so the returned Promise was always defined and the NotDetectedError branch
was unreachable — executable-presence detection always succeeded. The
executable-on-PATH fallback tier of package-manager detection therefore
unconditionally returned the highest-priority detector (pnpm), even on
machines without pnpm, e.g. offering 'pnpm install' in the checkly import
prompt.

Behavior change: in the fallback case (no lockfile, config file, user
agent, or runtime signal), a machine without pnpm now resolves to the
first installed of bun > deno > yarn > cnpm > npm instead of pnpm, or to
the final npm default when nothing is found. Machines with pnpm installed
are unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sorccu sorccu changed the title fix(cli): make executable presence detection actually probe PATH [RED-887] fix(cli): make executable presence detection actually probe PATH [RED-887] [ship] Aug 31, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved: ship/show PR from a same-repo branch.

@sorccu
sorccu merged commit 0476e62 into main Aug 31, 2026
17 checks passed
@sorccu
sorccu deleted the simo/red-887-detect-presence-missing-await branch August 31, 2026 08:59
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