From dc12b13ebea24901bf662060c24726df6ee11645 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Mon, 31 Aug 2026 16:59:15 +0900 Subject: [PATCH] fix(cli): make executable presence detection actually probe PATH [RED-887] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../package-files/__tests__/package-manager.spec.ts | 6 ++++++ .../check-parser/package-files/package-manager.ts | 9 +-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts b/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts index dbf945c7..d7f1f3e6 100644 --- a/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts +++ b/packages/cli/src/services/check-parser/package-files/__tests__/package-manager.spec.ts @@ -632,4 +632,10 @@ describe('PathLookup', () => { expect(await lookup.lookupPath('node')).toBeDefined() expect(await lookup.lookupPath('checkly-no-such-executable-xyz')).toBeUndefined() }) + + it('detects the presence of an executable on PATH and throws for one that is missing', async () => { + const lookup = new PathLookup() + await expect(lookup.detectPresence('node')).resolves.toBeUndefined() + await expect(lookup.detectPresence('checkly-no-such-executable-xyz')).rejects.toThrow() + }) }) diff --git a/packages/cli/src/services/check-parser/package-files/package-manager.ts b/packages/cli/src/services/check-parser/package-files/package-manager.ts index 2fce6bc9..041957f5 100644 --- a/packages/cli/src/services/check-parser/package-files/package-manager.ts +++ b/packages/cli/src/services/check-parser/package-files/package-manager.ts @@ -819,15 +819,8 @@ export class PathLookup { } } - // FIXME(RED-887): the missing `await` below means `foundPath` is a - // Promise — always defined — so this never throws and executable - // detection always "succeeds". Fixing it changes package-manager - // detection outcomes on machines that lack the executable, so it is - // tracked as a follow-up rather than fixed in passing; use lookupPath() - // for a working check. - // eslint-disable-next-line require-await async detectPresence (executable: string): Promise { - const foundPath = this.lookupPath(executable) + const foundPath = await this.lookupPath(executable) if (foundPath === undefined) { throw new NotDetectedError() }