From 63cb669c1494f0424c1b25af94fbf787e97f382f Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Tue, 11 Aug 2026 00:28:20 +0530 Subject: [PATCH 1/3] fix: ship web fetch-browser so the blocked-fetch escalation works (#247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `webcmd web fetch` raises FETCH_BLOCKED / FETCH_REQUIRES_BROWSER with a hint pointing at `webcmd web fetch-browser`, and smart-search names it as the mandatory second rung. On a fresh install that command died with ADAPTER_LOAD: 0470ac43 dropped "clis/" from package.json `files` when it moved adapters to plugins, but the generated cli-manifest.json still advertises web/fetch-browser, and no `web` plugin was ever created — so there was nothing to install either. The published package cannot carry adapter source at the root (the packaging guard forbids it), so the build stages clis/ next to the compiled output instead: dist/src/clis/ ships under the existing dist/src/ entry, and BUILTIN_CLIS resolves there when the repo-root tree is absent. The core manifest is staged alongside it so the manifest lookup contract (clisDir/../cli-manifest.json) still holds and an installed CLI does not fall back to a filesystem scan. check-package-bin now asserts every cli-manifest module is present in the tarball, which is the regression this bug was. Co-Authored-By: Claude Opus 5 --- scripts/check-package-bin.mjs | 16 ++++++++++++++++ scripts/copy-yaml.cjs | 13 ++++++++++++- src/build-manifest.ts | 9 ++++++++- src/main.ts | 10 +++++++--- 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/scripts/check-package-bin.mjs b/scripts/check-package-bin.mjs index 01c40594..48262f01 100644 --- a/scripts/check-package-bin.mjs +++ b/scripts/check-package-bin.mjs @@ -78,6 +78,22 @@ try { if (packedPaths.has('scripts/fetch-adapters.js')) { fail('packed tarball contains the retired adapter fetch lifecycle'); } + + // Every command the core manifest advertises must be in the tarball. A + // manifest entry whose module was left behind is discoverable but dies with + // ADAPTER_LOAD on first use — see #247, where web/fetch-browser was the only + // escalation path FETCH_BLOCKED knew how to recommend. + const manifest = JSON.parse(fs.readFileSync(path.join(ROOT, 'cli-manifest.json'), 'utf8')); + for (const entry of manifest) { + if (!entry.modulePath) continue; + const packedModule = `dist/src/clis/${entry.modulePath}`; + if (!packedPaths.has(packedModule)) { + fail(`packed tarball is missing manifest module for ${entry.site}/${entry.name}: ${packedModule}`); + } + } + if (manifest.length > 0 && !packedPaths.has('dist/src/cli-manifest.json')) { + fail('packed tarball is missing the staged core manifest: dist/src/cli-manifest.json'); + } for (const [name, target] of binEntries) { if (!packedPaths.has(String(target))) { fail(`packed tarball is missing bin "${name}" target: ${target}`); diff --git a/scripts/copy-yaml.cjs b/scripts/copy-yaml.cjs index 9af6dbfb..4b622bc4 100644 --- a/scripts/copy-yaml.cjs +++ b/scripts/copy-yaml.cjs @@ -2,7 +2,8 @@ * Copy YAML support files to dist/. * (Adapters are JS-first and no longer need yaml copying.) */ -const { copyFileSync, mkdirSync, existsSync } = require('fs'); +const { copyFileSync, cpSync, mkdirSync, existsSync } = require('fs'); +const { sep } = require('path'); // Copy external CLI registry to dist/ const extSrc = 'src/external-clis.yaml'; @@ -14,3 +15,13 @@ if (existsSync(extSrc)) { const playwrightClient = 'src/browser/run/generated/playwright-client.js'; mkdirSync('dist/src/browser/run/generated', { recursive: true }); copyFileSync(playwrightClient, 'dist/src/browser/run/generated/playwright-client.js'); + +// Stage the builtin adapter tree next to the compiled output. package.json +// `files` ships dist/src/ but not the repo-root clis/, so without this the +// core manifest points at modules the published package does not contain. +if (existsSync('clis')) { + cpSync('clis', 'dist/src/clis', { + recursive: true, + filter: (src) => !src.split(sep).includes('test'), + }); +} diff --git a/src/build-manifest.ts b/src/build-manifest.ts index 5cb93350..5fdc38bf 100644 --- a/src/build-manifest.ts +++ b/src/build-manifest.ts @@ -27,7 +27,7 @@ import * as path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { getErrorMessage } from './errors.js'; import { fullName, getRegistry, type CliCommand } from './registry.js'; -import { findPackageRoot } from './package-paths.js'; +import { findPackageRoot, getCliManifestPath } from './package-paths.js'; import type { ManifestEntry } from './manifest-types.js'; import { isRecord } from './utils.js'; import { @@ -450,6 +450,13 @@ async function main(): Promise { fs.mkdirSync(path.dirname(OUTPUT), { recursive: true }); fs.writeFileSync(OUTPUT, artifacts.manifestJson); + // The published package resolves the builtin tree to dist/src/clis/, and the + // manifest lookup is always clisDir/../cli-manifest.json — keep the staged + // copy in step so an installed CLI does not fall back to a filesystem scan. + const stagedClis = path.join(PACKAGE_ROOT, 'dist', 'src', 'clis'); + if (fs.existsSync(stagedClis)) { + fs.writeFileSync(getCliManifestPath(stagedClis), artifacts.manifestJson); + } fs.writeFileSync(HOSTED_CONTRACT_OUTPUT, artifacts.hostedContractJson); console.error(`✅ Manifest compiled: ${entries.length} entries → ${OUTPUT}`); console.error(`✅ Hosted contract compiled: ${packageMetadata.name}@${packageMetadata.version} → ${HOSTED_CONTRACT_OUTPUT}`); diff --git a/src/main.ts b/src/main.ts index 57fd60da..236e08a5 100644 --- a/src/main.ts +++ b/src/main.ts @@ -26,9 +26,13 @@ import { CONFIG_DIR_NAME } from './brand.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -// The empty core manifest remains next to the retired clis/ location so older -// user-local adapter manifests keep the same lookup contract. -const BUILTIN_CLIS = path.join(findPackageRoot(__filename), 'clis'); +// The core manifest sits next to the builtin clis/ tree so user-local adapter +// manifests keep the same lookup contract. In a repo checkout that tree is +// clis/ at the package root; the published package cannot ship adapter source +// at the root, so the build stages a copy next to the compiled output and this +// resolves to dist/src/clis/ there. +const REPO_BUILTIN_CLIS = path.join(findPackageRoot(__filename), 'clis'); +const BUILTIN_CLIS = fs.existsSync(REPO_BUILTIN_CLIS) ? REPO_BUILTIN_CLIS : path.join(__dirname, 'clis'); const USER_CLIS = path.join(os.homedir(), CONFIG_DIR_NAME, 'clis'); const USER_PLUGINS = path.join(os.homedir(), CONFIG_DIR_NAME, 'plugins'); From b58d352e26229151d11d3c728c55f2703598bc9e Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Wed, 12 Aug 2026 01:01:59 +0530 Subject: [PATCH 2/3] test: prove manifest commands load from the installed package layout check:package-bin already asserted every manifest module is in the tarball; presence is not executability. Load each one from the npm-installed prefix through the CLI's own builtin-clis resolver, which is the branch a repo checkout never takes and the one that produced ADAPTER_LOAD in #247. The resolver moves to package-paths so the check exercises the real thing rather than a copy that can drift. Co-Authored-By: Claude Opus 5 --- scripts/check-package-bin.mjs | 29 ++++++++++++++++++++++++++++- src/main.ts | 10 +++------- src/package-paths.ts | 14 ++++++++++++++ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/scripts/check-package-bin.mjs b/scripts/check-package-bin.mjs index 48262f01..ae304b57 100644 --- a/scripts/check-package-bin.mjs +++ b/scripts/check-package-bin.mjs @@ -4,7 +4,7 @@ import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { formatPackageBinSpawnFailure, packageBinSpawnOptions, @@ -112,6 +112,33 @@ try { fail(`global install did not create executable: ${binPath}`); } run(binPath, ['--version'], { cwd: tmp }); + // Presence in the tarball is not executability. Every manifest command must + // also be discoverable and loadable *from the installed layout*, which a + // repository checkout never exercises: there the builtin tree is clis/ at + // the package root, and only the installed package takes the dist/src/clis + // fallback. #247 was exactly this gap — advertised, then ADAPTER_LOAD on + // first use. + const installedMain = path.join(prefix, 'lib', 'node_modules', pkg.name, pkg.main); + for (const entry of manifest) { + if (!entry.modulePath) continue; + const help = run(binPath, [entry.site, entry.name, '--help'], { cwd: tmp }); + if (/ADAPTER_LOAD/.test(`${help.stdout}${help.stderr}`)) { + fail(`installed package cannot present ${entry.site}/${entry.name}: ADAPTER_LOAD\n${help.stdout}${help.stderr}`); + } + // --help is served from the staged manifest and never imports the module; + // the import below is what execution does, through the CLI's own resolver. + const load = [ + `const { resolveBuiltinClisDir } = await import(${JSON.stringify(pathToFileURL(path.join(path.dirname(installedMain), 'package-paths.js')).href)});`, + `const { pathToFileURL } = await import('node:url');`, + `const { join } = await import('node:path');`, + `const dir = resolveBuiltinClisDir(${JSON.stringify(installedMain)});`, + `await import(pathToFileURL(join(dir, ${JSON.stringify(entry.modulePath)})).href);`, + ].join('\n'); + const result = spawnSync(process.execPath, ['--input-type=module', '-e', load], { cwd: tmp, encoding: 'utf8' }); + if (result.status !== 0) { + fail(`installed package cannot load ${entry.site}/${entry.name} (${entry.modulePath}) — this is the ADAPTER_LOAD failure users hit:\n${result.stderr}`); + } + } } } finally { fs.rmSync(tmp, { recursive: true, force: true }); diff --git a/src/main.ts b/src/main.ts index 236e08a5..e7e645aa 100644 --- a/src/main.ts +++ b/src/main.ts @@ -18,7 +18,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; import { getCompletionScriptFast, getCompletionsFromManifest, hasAllManifests } from './completion-fast.js'; -import { findPackageRoot, getCliManifestPath } from './package-paths.js'; +import { findPackageRoot, getCliManifestPath, resolveBuiltinClisDir } from './package-paths.js'; import { PKG_VERSION } from './version.js'; import { EXIT_CODES } from './errors.js'; import { isSupportedNodeVersion, MIN_SUPPORTED_NODE_MAJOR } from './runtime-detect.js'; @@ -27,12 +27,8 @@ import { CONFIG_DIR_NAME } from './brand.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); // The core manifest sits next to the builtin clis/ tree so user-local adapter -// manifests keep the same lookup contract. In a repo checkout that tree is -// clis/ at the package root; the published package cannot ship adapter source -// at the root, so the build stages a copy next to the compiled output and this -// resolves to dist/src/clis/ there. -const REPO_BUILTIN_CLIS = path.join(findPackageRoot(__filename), 'clis'); -const BUILTIN_CLIS = fs.existsSync(REPO_BUILTIN_CLIS) ? REPO_BUILTIN_CLIS : path.join(__dirname, 'clis'); +// manifests keep the same lookup contract. +const BUILTIN_CLIS = resolveBuiltinClisDir(__filename); const USER_CLIS = path.join(os.homedir(), CONFIG_DIR_NAME, 'clis'); const USER_PLUGINS = path.join(os.homedir(), CONFIG_DIR_NAME, 'plugins'); diff --git a/src/package-paths.ts b/src/package-paths.ts index 379c0410..3c6d1050 100644 --- a/src/package-paths.ts +++ b/src/package-paths.ts @@ -49,6 +49,20 @@ export function getBuiltEntryCandidates( return [...new Set(candidates)]; } +/** + * Directory holding the builtin adapter modules, given the running entry file. + * + * In a repo checkout that is `clis/` at the package root. The published package + * cannot ship adapter source at the root, so the build stages a copy next to the + * compiled output and this resolves to `dist/src/clis/` there. Shared so the + * packaging check can load manifest modules through the same resolution the CLI + * uses at execution time, rather than a copy of it that can drift. + */ +export function resolveBuiltinClisDir(entryFile: string, fileExists: (candidate: string) => boolean = fs.existsSync): string { + const repoClis = path.join(findPackageRoot(entryFile, fileExists), 'clis'); + return fileExists(repoClis) ? repoClis : path.join(path.dirname(entryFile), 'clis'); +} + export function getCliManifestPath(clisDir: string): string { return path.resolve(clisDir, '..', 'cli-manifest.json'); } From d081bb98abbc365cbb7f7d5941e64054fc7adc12 Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Wed, 12 Aug 2026 01:11:09 +0530 Subject: [PATCH 3/3] fix: resolve the installed package root on Windows too npm -g --prefix installs to /node_modules on Windows, not /lib/node_modules, and check:package-bin runs on a Windows runner. Co-Authored-By: Claude Opus 5 --- scripts/check-package-bin.mjs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/check-package-bin.mjs b/scripts/check-package-bin.mjs index ae304b57..09227281 100644 --- a/scripts/check-package-bin.mjs +++ b/scripts/check-package-bin.mjs @@ -118,7 +118,15 @@ try { // the package root, and only the installed package takes the dist/src/clis // fallback. #247 was exactly this gap — advertised, then ADAPTER_LOAD on // first use. - const installedMain = path.join(prefix, 'lib', 'node_modules', pkg.name, pkg.main); + // npm -g --prefix puts packages under /lib/node_modules on POSIX + // and directly under /node_modules on Windows. + const installedRoot = process.platform === 'win32' + ? path.join(prefix, 'node_modules', pkg.name) + : path.join(prefix, 'lib', 'node_modules', pkg.name); + const installedMain = path.join(installedRoot, pkg.main); + if (!fs.existsSync(installedMain)) { + fail(`global install did not create the package entry point: ${installedMain}`); + } for (const entry of manifest) { if (!entry.modulePath) continue; const help = run(binPath, [entry.site, entry.name, '--help'], { cwd: tmp });