From fc3079505608716ee346bad5926b2f6306f98139 Mon Sep 17 00:00:00 2001 From: Vojtech Novak Date: Fri, 30 Oct 2026 12:04:58 +0100 Subject: [PATCH] [agent-cli] install with no arguments installs the lockfile when expo is not installed yet A bare `install` in a project whose `expo` package does not resolve runs the lockfile's frozen install (npm ci, bun install --frozen-lockfile, pnpm install --frozen-lockfile, yarn install) in the lockfile's directory, then syncs skills and exits with the package manager's code. `npx expo install` cannot read the SDK of a project with no node_modules and fails with "ConfigError: expo not installed". A named package in that state is refused with DEPENDENCIES_NOT_INSTALLED, and the suggested command is the bare install. A typecheck with no compiler suggests the bare install when package.json declares typescript. Red before: e2e install-bootstrap-test (4 cases) and the new typecheck-test case. Co-Authored-By: Claude Opus 5.5 --- .../e2e/__tests__/install-bootstrap-test.ts | 89 +++++++++++++++++++ .../agent-cli/e2e/__tests__/typecheck-test.ts | 16 ++++ .../llp/0010-agent-conventions.rfc.md | 4 +- .../install/__tests__/installAsync-test.ts | 8 ++ .../agent-cli/src/install/installAsync.ts | 65 +++++++++++++- .../agent-cli/src/typecheck/checkAsync.ts | 32 ++++++- .../src/utils/__tests__/packageRunner-test.ts | 51 +++++++++++ .../agent-cli/src/utils/packageRunner.ts | 64 +++++++++++-- 8 files changed, 313 insertions(+), 16 deletions(-) create mode 100644 packages/@expo/agent-cli/e2e/__tests__/install-bootstrap-test.ts diff --git a/packages/@expo/agent-cli/e2e/__tests__/install-bootstrap-test.ts b/packages/@expo/agent-cli/e2e/__tests__/install-bootstrap-test.ts new file mode 100644 index 000000000..1b4f3bc26 --- /dev/null +++ b/packages/@expo/agent-cli/e2e/__tests__/install-bootstrap-test.ts @@ -0,0 +1,89 @@ +// @ref llp/0010-agent-conventions.rfc.md §Forwarded codes, and the exception +// +// A fresh clone has no node_modules, so `expo install` cannot read the SDK. A bare `install` then +// runs the lockfile's frozen install, and a named package is refused until it has. +import fs from 'node:fs'; +import path from 'node:path'; + +import { + executeAgentCliAsync, + getTemporaryPath, + installStubBinAsync, + pathEnvVars, + setupFixtureAsync, + stubExpoEnv, +} from '../utils'; + +const STUB_PACKAGE_MANAGER = `const fs = require('fs'); +fs.appendFileSync(process.env.STUB_PM_LOG, JSON.stringify({ args: process.argv.slice(2), cwd: process.cwd() }) + '\\n'); +process.exit(Number(process.env.STUB_PM_EXIT_CODE ?? 0)); +`; + +/** A go-app copy with no installed `expo`, and stub `npm` and `bun` bins that record each call. */ +async function setupUninstalledProjectAsync(lockfile: string) { + const projectRoot = await setupFixtureAsync('go-app'); + await fs.promises.rm(path.join(projectRoot, 'node_modules', 'expo'), { recursive: true }); + await fs.promises.writeFile(path.join(projectRoot, lockfile), ''); + + const binDir = path.join(getTemporaryPath(), 'pm-bin'); + const script = path.join(binDir, 'stub-pm.js'); + await fs.promises.mkdir(binDir, { recursive: true }); + await fs.promises.writeFile(script, STUB_PACKAGE_MANAGER); + for (const name of ['npm', 'bun']) { + await installStubBinAsync(binDir, name, script); + } + const log = path.join(binDir, 'calls.jsonl'); + const env = { + ...pathEnvVars(`${binDir}${path.delimiter}${stubExpoEnv(projectRoot).PATH}`), + STUB_PM_LOG: log, + }; + const calls = (): { args: string[]; cwd: string }[] => + fs.existsSync(log) + ? fs + .readFileSync(log, 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)) + : []; + return { projectRoot, env, calls }; +} + +describe('@expo/agent-cli install with no installed dependencies', () => { + it.each([ + ['package-lock.json', ['npm', 'ci']], + ['bun.lock', ['bun', 'install', '--frozen-lockfile']], + ])(`should install %s's dependencies on a bare install`, async (lockfile, argv) => { + const { projectRoot, env, calls } = await setupUninstalledProjectAsync(lockfile); + + await executeAgentCliAsync(projectRoot, ['install'], { env }); + + expect(calls()).toEqual([{ args: argv.slice(1), cwd: fs.realpathSync(projectRoot) }]); + }); + + it(`should exit with the package manager's code when its install fails`, async () => { + const { projectRoot, env } = await setupUninstalledProjectAsync('package-lock.json'); + + const result = await executeAgentCliAsync(projectRoot, ['install'], { + env: { ...env, STUB_PM_EXIT_CODE: '3' }, + reject: false, + }); + + expect(result.exitCode).toBe(3); + }); + + it(`should refuse a named package and name the bare install`, async () => { + const { projectRoot, env, calls } = await setupUninstalledProjectAsync('package-lock.json'); + + const result = await executeAgentCliAsync(projectRoot, ['install', 'expo-sqlite', '--json'], { + env, + reject: false, + }); + + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stdout).error).toMatchObject({ + code: 'DEPENDENCIES_NOT_INSTALLED', + suggestedCommand: 'npx @expo/agent-cli install', + }); + expect(calls()).toEqual([]); + }); +}); diff --git a/packages/@expo/agent-cli/e2e/__tests__/typecheck-test.ts b/packages/@expo/agent-cli/e2e/__tests__/typecheck-test.ts index 5e7549c4f..4963f9a39 100644 --- a/packages/@expo/agent-cli/e2e/__tests__/typecheck-test.ts +++ b/packages/@expo/agent-cli/e2e/__tests__/typecheck-test.ts @@ -262,6 +262,22 @@ describe('@expo/agent-cli typecheck', () => { expect(error.message).not.toContain('nothing to type-check'); }); + it(`should suggest the bare install when package.json declares typescript and node_modules is empty`, async () => { + const projectRoot = await setupFixtureAsync('go-app'); + await fs.promises.writeFile(path.join(projectRoot, 'tsconfig.json'), '{}'); + const manifestPath = path.join(projectRoot, 'package.json'); + const manifest = JSON.parse(await fs.promises.readFile(manifestPath, 'utf8')); + manifest.devDependencies = { ...manifest.devDependencies, typescript: '~5.9.0' }; + await fs.promises.writeFile(manifestPath, JSON.stringify(manifest)); + + const result = await executeAgentCliAsync(projectRoot, ['typecheck', '--json'], { + reject: false, + }); + + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stdout).error.suggestedCommand).toBe('npx @expo/agent-cli install'); + }); + // A compiler that failed without reporting a diagnostic has not answered the question, so it is // the tool failing and not a verdict on the code — exit 1, with the envelope. it(`should exit 1 when the compiler failed without reporting anything`, async () => { diff --git a/packages/@expo/agent-cli/llp/0010-agent-conventions.rfc.md b/packages/@expo/agent-cli/llp/0010-agent-conventions.rfc.md index 258dd41ed..8b90fc7aa 100644 --- a/packages/@expo/agent-cli/llp/0010-agent-conventions.rfc.md +++ b/packages/@expo/agent-cli/llp/0010-agent-conventions.rfc.md @@ -42,7 +42,7 @@ No-dev-server and wrong-platform stay `1`. Neither becomes different within a se ### Forwarded codes, and the exception -`install`, `start`, `dev`, and the `expo` passthrough hand back whatever the subprocess exited with, verbatim. A wrapper's own failures use the table. +`install`, `start`, `dev`, and the `expo` passthrough hand back whatever the subprocess exited with, verbatim. A wrapper's own failures use the table. A bare `install` in a project whose `expo` package does not resolve installs the lockfile first (`npm ci`, `bun install --frozen-lockfile`, `pnpm install --frozen-lockfile` or `yarn install`, in the lockfile's directory) and exits with its code. The exception is a command that adds a verdict. `doctor` is `0` when every check passed, `20` when any failed, `1` only when the run produced no verdict ([[0016-v1-scope]] §Doctor's exit code). expo-doctor's own code stays on the `--json` `exitCode` field. @@ -62,7 +62,7 @@ No `eas --json` payload can contain a `null`. `printJsonOnlyOutput` deletes ever A project with no TypeScript exits `0`, with `checked: false`. `reason` is present exactly when `checked` is false. The follow-up says so in a command as well as in a field, so "nothing was checked" cannot read as "everything passed". -A TypeScript project with no compiler is `1` (`TYPECHECK_CLI_MISSING`), with `Try: npx @expo/agent-cli install typescript --dev`. Either `tsconfig.json` or `.ts`/`.tsx` sources makes it a TypeScript project. `.d.ts` files alone do not, since `expo-env.d.ts` is generated into every app. +A TypeScript project with no compiler is `1` (`TYPECHECK_CLI_MISSING`), with `Try: npx @expo/agent-cli install typescript --dev`. When package.json declares typescript, the Try line is the bare `install`. Either `tsconfig.json` or `.ts`/`.tsx` sources makes it a TypeScript project. `.d.ts` files alone do not, since `expo-env.d.ts` is generated into every app. No compiler is ever fetched. `doctor:check` falls back to `npx expo-doctor`. This command does not. A type check is a function of the project's own compiler version, its `tsconfig.json`, and its `@types`. Only `node_modules/.bin/tsc` counts. diff --git a/packages/@expo/agent-cli/src/install/__tests__/installAsync-test.ts b/packages/@expo/agent-cli/src/install/__tests__/installAsync-test.ts index e854d97b3..58fe6582a 100644 --- a/packages/@expo/agent-cli/src/install/__tests__/installAsync-test.ts +++ b/packages/@expo/agent-cli/src/install/__tests__/installAsync-test.ts @@ -1,3 +1,6 @@ +import { vol } from 'memfs'; +import path from 'path'; + import * as Log from '../../log'; import type { InstallImpactReport } from '../../project/types'; import { @@ -38,6 +41,11 @@ function printed(): string { } beforeEach(() => { + // A project whose dependencies are installed: `installAsync` runs `expo install` only then. + vol.reset(); + vol.fromJSON({ + [path.join(projectRoot, 'node_modules', 'expo', 'package.json')]: '{"name":"expo"}', + }); // `clearMocks` empties the call log but keeps implementations, so every mock whose *answer* // matters is given its default back here — otherwise one test's answer leaks into the next. vi.mocked(runExpoAsync).mockResolvedValue(0); diff --git a/packages/@expo/agent-cli/src/install/installAsync.ts b/packages/@expo/agent-cli/src/install/installAsync.ts index 1ea83f696..508595fef 100644 --- a/packages/@expo/agent-cli/src/install/installAsync.ts +++ b/packages/@expo/agent-cli/src/install/installAsync.ts @@ -3,12 +3,18 @@ import { buildInstallFollowUps, followUpsEnabled, reportFollowUps } from '../fol import type { FollowUp } from '../followups/types'; import * as Log from '../log'; import type { InstallImpactReport } from '../project/types'; +import { PROGRAM_PREFIX } from '../programName'; import { autoSyncSkillsAsync, listSkillPackagesAsync, printSkillsForAgentAsync, } from '../skills/skillsAsync'; +import { resolvePackageRootSync } from '../project/nodeModules'; +import { CommandError } from '../utils/errors'; import { runExpoAsync, spawnExpoAsync } from '../utils/expoCli'; +import { runInheritedAsync } from '../utils/inheritedRun'; +import { lockfileInstall } from '../utils/packageRunner'; +import { spawnSubprocessAsync } from '../utils/subprocess'; import { diagnoseCheckedPackagesAsync, type InstallCheckReport } from './checkReport'; import { reportInstallImpactAsync } from './impactReport'; import type { InstallPlan } from './resolveOptions'; @@ -25,7 +31,16 @@ import type { InstallReport } from './types'; * @returns the exit code of the `expo install` subprocess. */ export async function installAsync(projectRoot: string, plan: InstallPlan): Promise { - const { exitCode, checkPayload, checkOutput } = await runInstallAsync(projectRoot, plan); + const dependenciesMissing = + !plan.check && + !plan.expoArgs.includes('--fix') && + resolvePackageRootSync(projectRoot, 'expo') == null; + if (dependenciesMissing && plan.packages.length > 0) { + throw dependenciesNotInstalledError(); + } + const { exitCode, checkPayload, checkOutput } = dependenciesMissing + ? await runLockfileInstallAsync(projectRoot, plan) + : await runInstallAsync(projectRoot, plan); if (exitCode !== 0) { // Nothing was installed, so there is nothing to classify and nothing to link — but a caller // that asked for JSON still gets one object, the way a successful run does. @@ -64,6 +79,54 @@ export async function installAsync(projectRoot: string, plan: InstallPlan): Prom return exitCode; } +/** + * Install the lockfile's dependencies, for a project whose `expo` does not resolve yet. + * + * `expo install` cannot do this: the `expo` it would run is fetched by the package runner, and it + * cannot read the SDK of a project with no `node_modules`. The package manager's own frozen install + * can, so it runs, in the lockfile's directory. Output follows {@link runInstallAsync}: inherited + * for a person, captured and sent to stderr under `--json`. + * + * @ref llp/0010-agent-conventions.rfc.md §Forwarded codes, and the exception + */ +async function runLockfileInstallAsync( + projectRoot: string, + plan: InstallPlan +): Promise<{ exitCode: number; checkPayload: unknown; checkOutput: string | null }> { + const { lockfile, cwd, argv } = lockfileInstall(projectRoot); + const [command, ...args] = argv as [string, ...string[]]; + const message = `Installing the dependencies of ${lockfile ?? 'package.json'} with ${argv.join(' ')}`; + + if (!plan.json) { + Log.log(message); + return { + exitCode: await runInheritedAsync(command, args, { cwd }), + checkPayload: null, + checkOutput: null, + }; + } + Log.error(message); + const result = await spawnSubprocessAsync(command, args, { cwd, output: 'capture' }); + const printed = `${result.stdout}${result.stderr}`.trim(); + if (printed) { + Log.error(printed); + } + return { exitCode: result.exitCode ?? 1, checkPayload: null, checkOutput: null }; +} + +function dependenciesNotInstalledError(): CommandError { + const error = new CommandError( + 'DEPENDENCIES_NOT_INSTALLED', + [ + `This project has no installed dependencies, so no package was added.`, + `Why: the expo package does not resolve from this project or any directory above it, and "expo install" cannot read the SDK of a project with no node_modules.`, + `How: run "${PROGRAM_PREFIX} install" with no packages first. It installs the lockfile. Then add the package.`, + ].join('\n') + ); + error.suggestedCommand = `${PROGRAM_PREFIX} install`; + return error; +} + /** * Run `expo install`, and read the `--check` report out of it when there is one. * diff --git a/packages/@expo/agent-cli/src/typecheck/checkAsync.ts b/packages/@expo/agent-cli/src/typecheck/checkAsync.ts index 3eec27351..d36acf2cb 100644 --- a/packages/@expo/agent-cli/src/typecheck/checkAsync.ts +++ b/packages/@expo/agent-cli/src/typecheck/checkAsync.ts @@ -8,6 +8,11 @@ import fs from 'fs'; import path from 'path'; import { stripVTControlCharacters } from 'util'; +import { + listDependencyNames, + readJsonFileSync, + type ProjectPackageJson, +} from '../project/nodeModules'; import { PROGRAM_PREFIX } from '../programName'; import { fileExistsSync } from '../utils/dir'; import { CommandError } from '../utils/errors'; @@ -161,7 +166,11 @@ export async function runTypeCheckAsync(projectRoot: string): Promise(path.join(projectRoot, 'package.json')); + return listDependencyNames(manifest).includes('typescript'); +} + /** The error for a compiler that is on disk and could not be started. */ function notRunnableError(cli: TypeScriptCli, why: string): CommandError { const error = new CommandError( diff --git a/packages/@expo/agent-cli/src/utils/__tests__/packageRunner-test.ts b/packages/@expo/agent-cli/src/utils/__tests__/packageRunner-test.ts index 82fadda9d..cb6c76393 100644 --- a/packages/@expo/agent-cli/src/utils/__tests__/packageRunner-test.ts +++ b/packages/@expo/agent-cli/src/utils/__tests__/packageRunner-test.ts @@ -11,6 +11,7 @@ import path from 'path'; import { resetInvokerCache } from '../invoker'; import { + lockfileInstall, packageRunnerLabel, resetPackageRunnerCache, resolvePackageRunner, @@ -83,3 +84,53 @@ describe(packageRunnerLabel, () => { expect(packageRunnerLabel({ runner: 'npx', command: 'npx' })).toBe('npx'); }); }); + +describe(lockfileInstall, () => { + const workspace = path.resolve('/workspace'); + const app = path.join(workspace, 'apps', 'mobile'); + + it(`should prefer bun over npm in one directory`, () => { + vol.fromJSON({ + [path.join(workspace, 'bun.lock')]: '', + [path.join(workspace, 'package-lock.json')]: '{}', + }); + + expect(lockfileInstall(workspace)).toMatchObject({ + manager: 'bun', + cwd: workspace, + argv: ['bun', 'install', '--frozen-lockfile'], + }); + }); + + it(`should use the workspace root's lockfile for a package of it`, () => { + vol.fromJSON({ + [path.join(workspace, 'package-lock.json')]: '{}', + [path.join(app, 'package.json')]: '{}', + }); + + expect(lockfileInstall(app)).toMatchObject({ + manager: 'npm', + cwd: workspace, + argv: ['npm', 'ci'], + }); + }); + + it.each([ + ['pnpm-lock.yaml', ['pnpm', 'install', '--frozen-lockfile']], + ['yarn.lock', ['yarn', 'install']], + ])(`should read %s`, (file, argv) => { + vol.fromJSON({ [path.join(workspace, file)]: '' }); + + expect(lockfileInstall(workspace).argv).toEqual(argv); + }); + + it(`should run npm install in the project when there is no lockfile`, () => { + vol.fromJSON({ [path.join(app, 'package.json')]: '{}' }); + + expect(lockfileInstall(app)).toMatchObject({ + manager: 'npm', + cwd: app, + argv: ['npm', 'install'], + }); + }); +}); diff --git a/packages/@expo/agent-cli/src/utils/packageRunner.ts b/packages/@expo/agent-cli/src/utils/packageRunner.ts index 12360403e..4b2b7f79c 100644 --- a/packages/@expo/agent-cli/src/utils/packageRunner.ts +++ b/packages/@expo/agent-cli/src/utils/packageRunner.ts @@ -98,31 +98,77 @@ const LOCKFILES: { manager: string; files: string[] }[] = [ { manager: 'pnpm', files: ['pnpm-lock.yaml'] }, ]; -/** - * Whether the lockfile that governs this project is bun's. - * - * Walks up, because a package of a monorepo has no lockfile of its own and the workspace root's is - * the one that installed it. The first directory holding *any* known lockfile decides. - */ -export function projectUsesBun(projectRoot: string): boolean { +/** The lockfile that governs a project, the directory it is in, and the manager it belongs to. */ +function findLockfile(projectRoot: string): { manager: string; file: string; dir: string } | null { let dir = path.resolve(projectRoot); // Bounded by the filesystem root: `path.dirname('/')` is `'/'`, which ends the walk. for (;;) { for (const { manager, files } of LOCKFILES) { for (const file of files) { if (fileExistsSync(path.join(dir, file))) { - return manager === 'bun'; + return { manager, file, dir }; } } } const parent = path.dirname(dir); if (parent === dir) { - return false; + return null; } dir = parent; } } +/** + * Whether the lockfile that governs this project is bun's. + * + * Walks up, because a package of a monorepo has no lockfile of its own and the workspace root's is + * the one that installed it. The first directory holding *any* known lockfile decides. + */ +export function projectUsesBun(projectRoot: string): boolean { + return findLockfile(projectRoot)?.manager === 'bun'; +} + +/** The command that installs a project's dependencies exactly as its lockfile records them. */ +export interface LockfileInstall { + manager: string; + /** The lockfile's name, or null when the project has none. */ + lockfile: string | null; + /** The lockfile's directory (a workspace's root), or the project when there is no lockfile. */ + cwd: string; + /** The command and its arguments. */ + argv: string[]; +} + +const LOCKFILE_INSTALL_ARGV: Record = { + bun: ['bun', 'install', '--frozen-lockfile'], + pnpm: ['pnpm', 'install', '--frozen-lockfile'], + yarn: ['yarn', 'install'], + npm: ['npm', 'ci'], +}; + +/** + * What installs this project's dependencies from its lockfile, found the way {@link projectUsesBun} + * finds the lockfile: the nearest directory above holding one. A project with none gets + * `npm install`, because `npm ci` refuses to run without a lockfile. + */ +export function lockfileInstall(projectRoot: string): LockfileInstall { + const found = findLockfile(projectRoot); + if (!found) { + return { + manager: 'npm', + lockfile: null, + cwd: path.resolve(projectRoot), + argv: ['npm', 'install'], + }; + } + return { + manager: found.manager, + lockfile: found.file, + cwd: found.dir, + argv: LOCKFILE_INSTALL_ARGV[found.manager]!, + }; +} + /** * The runner a *project's* published packages should be spawned with. *