Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/not-installed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"uptide": patch
---

`fix` and `check` in a checkout whose dependencies are not installed now stop before any analysis with `PACKAGE_NOT_INSTALLED` (exit 2), name the `package.json` that declares the package, say whether nothing is installed or only that package is missing, and give the frozen install for the lockfile (`npm ci`, `pnpm install --frozen-lockfile`, `yarn install --frozen-lockfile`, `yarn install --immutable`). Before, a `node_modules` in a directory above the project counted as the project's install, so a fresh clone under such a directory got past the check and `fix` failed with a misleading "has no upgrade". Uptide still installs nothing in your checkout. `check --json` now answers every refusal on stdout with `{"error": {"code", "message"}}`, as `fix --json` does. When the package is already at or past the target, `fix` says "<package> is already at <version> (target <target>)".
2 changes: 1 addition & 1 deletion docs/commands/check.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ whole-repository budgeted mode or `--max-time` flag. Install dependencies first.
| `--all-deps` | also analyze `@types/*` and dependencies the repository never imports |
| `--html [path]`, `--open` | a shareable report (below) |
| `--cwd <dir>` | repository to work on |
| `--json` | the full report as JSON on stdout |
| `--json` | the full report as JSON on stdout; when the run stops before a report, `{"error": {"code", "message"}}` instead (exit 2) |
| `--ci` | plain log output: no color, no spinner (`NO_COLOR` is respected too) |

While it runs, a terminal shows one live line with the current phase, which disappears when
Expand Down
13 changes: 13 additions & 0 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ classic/Berry with the node-modules linker. Deno is not supported.
lockfile` once (bun's own flag) so the repository has `bun.lock`, and `check` can read the
installed versions.

## "declared in package.json but not installed"

`check` and `fix` compile your code against the version installed in `node_modules`, so
they stop before any analysis when the package is not there, with the code `PACKAGE_NOT_INSTALLED`
(exit 2; `--json` carries it in `error.code`). "nothing is installed (no node_modules)"
means a fresh clone: install the dependencies, then run the same command again. "node_modules
is there, without it" means a partial install, for example a workspace package installed on
its own. The message names the `package.json` that declares the package, and the next
command is the frozen install for the repository's lockfile: `npm ci`, `pnpm install
--frozen-lockfile`, `yarn install --frozen-lockfile` or `yarn install --immutable`. Uptide
never installs into your checkout. Only the project's own `node_modules` counts: a copy in a
directory above the project root (a stray `~/node_modules`) is not its install.

## Package manager not available, or the wrong version

`fix` runs the package manager the repository names in `packageManager` (corepack's choice),
Expand Down
36 changes: 36 additions & 0 deletions packages/cli/src/__snapshots__/failures.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`friendly failures: each says what to run next > a copy above the project root is not its install (UPT-136) 1`] = `
[
" Next: pnpm install --frozen-lockfile",
]
`;

exports[`friendly failures: each says what to run next > a dependency asked for by name that is not there: exit 2 1`] = `
[
" Next: npx uptide",
]
`;

exports[`friendly failures: each says what to run next > a partial install names the package and the workspace that declares it 1`] = `
[
" Next: pnpm install --frozen-lockfile",
]
`;

exports[`friendly failures: each says what to run next > a registry error thrown by the engine is the same failure 1`] = `
[
" Next: run the same command again in a minute",
Expand All @@ -16,6 +28,18 @@ exports[`friendly failures: each says what to run next > a repository without zo

exports[`friendly failures: each says what to run next > breaking found and a dependency failed: breaking is the answer (exit 1) 1`] = `[]`;

exports[`friendly failures: each says what to run next > check --json answers a refusal with its code on stdout 1`] = `
[
" Next: pnpm install --frozen-lockfile",
]
`;

exports[`friendly failures: each says what to run next > check --json answers a refusal with its code on stdout 2`] = `
[
" Next: npx uptide --json",
]
`;

exports[`friendly failures: each says what to run next > declared but missing from the lockfile 1`] = `
[
" Next: npm install",
Expand All @@ -42,6 +66,18 @@ exports[`friendly failures: each says what to run next > fix outside git, below
]
`;

exports[`friendly failures: each says what to run next > fix stops before any work when nothing is installed 1`] = `
[
" Next: pnpm install --frozen-lockfile",
]
`;

exports[`friendly failures: each says what to run next > fix stops before any work when nothing is installed 2`] = `
[
" Next: pnpm install --frozen-lockfile",
]
`;

exports[`friendly failures: each says what to run next > missing ANTHROPIC_API_KEY: fix still runs with the rules 1`] = `
[
" Next: npx uptide fix zod (set ANTHROPIC_API_KEY, OPENAI_API_KEY or GEMINI_API_KEY in your environment first)",
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export async function run(
} else {
if (err instanceof CliError && err.options.next)
err.options.next = suggest(err.options.next);
// `fix --json` answers on every exit, a failure included (UPT-107).
// `fix --json` and `check --json` answer on every exit, a failure included (UPT-107, UPT-136).
if (flags.json && options.jsonErrors)
emit({
...(err instanceof ReportedError ? err.report : {}),
Expand Down Expand Up @@ -736,7 +736,7 @@ ${EXIT_CODES(
}
return EXIT.ok;
},
{ quiet: !flags.verbose },
{ quiet: !flags.verbose, jsonErrors: true },
),
);

Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
type ErrorCode,
IntegrityError,
NoTypesError,
PackageNotFoundError,
Expand All @@ -13,16 +14,20 @@ export const EXIT = { ok: 0, breaking: 1, error: 2 } as const;
/** A failure the user can act on. `next` is the exact command to run, when there is one. */
export class CliError extends Error {
override readonly name = 'CliError';
/** What `--json` reports; ANALYSIS_FAILED when unset. */
readonly code: ErrorCode | undefined;
constructor(
message: string,
readonly options: {
why?: string;
next?: string;
literalNext?: boolean;
exitCode?: number;
code?: ErrorCode;
} = {},
) {
super(message);
this.code = options.code;
}
}

Expand Down
77 changes: 75 additions & 2 deletions packages/cli/src/failures.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { writeFileSync } from 'node:fs';
import { rmSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';
import { type PackageReport, UptideError } from '@uptide/core';
import { describe, expect, it } from 'vitest';
Expand Down Expand Up @@ -86,11 +86,84 @@ describe('friendly failures: each says what to run next', () => {
);
expect(code).toBe(2);
expect(stderr).toContain('✖ Dependencies');
expect(stderr).toContain('error: zod and stripe are in the lockfile but not installed');
expect(stderr).toContain(
'error: zod and stripe are declared in package.json but not installed: nothing is installed (no node_modules)',
);
expect(stderr).toContain(`Next: ${next}\n`);
expect(engine.calls).toEqual([]);
});

it('a partial install names the package and the workspace that declares it', async () => {
const cwd = tempRepo({
'package.json': '{"name":"mono"}',
'pnpm-lock.yaml': '',
'packages/api/package.json': manifest,
'node_modules/stripe/package.json': '{}',
});
const engine = fakeEngine({
workspaces: async () => ['.', 'packages/api'],
declared: async (dir) =>
new Map(dir.endsWith('api') ? [['zod', '^3.23.8']] : [['stripe', '14.25.0']]),
});
const { code, stderr } = await fail(['check', 'zod'], cwd, engine);
expect(code).toBe(2);
expect(stderr).toContain(
'error: zod is declared in packages/api/package.json but not installed: node_modules is there, without it',
);
expect(stderr).toContain('Next: pnpm install --frozen-lockfile\n');
expect(engine.calls).toEqual([]);
});

it('a copy above the project root is not its install (UPT-136)', async () => {
const parent = tempRepo({
'node_modules/zod/package.json': '{"name":"zod","version":"4.6.5"}',
'shop/package.json': manifest,
'shop/pnpm-lock.yaml': '',
});
const { code, stderr } = await fail(['check', 'zod'], join(parent, 'shop'));
expect(code).toBe(2);
expect(stderr).toContain(
'error: zod is declared in package.json but not installed: nothing is installed (no node_modules)',
);
});

it('fix stops before any work when nothing is installed', async () => {
const root = pnpmGitRepo();
rmSync(join(root, 'node_modules'), { recursive: true });
const { code, stderr, engine } = await fail(['fix', '--only', 'zod'], root);
expect(code).toBe(2);
expect(stderr).toContain('✖ Repository');
expect(stderr).toContain(
'error: zod is declared in package.json but not installed: nothing is installed (no node_modules)',
);
expect(stderr).toContain('Next: pnpm install --frozen-lockfile\n');
expect(engine.calls).toEqual([]);
const json = await fail(['fix', '--only', 'zod', '--json'], root);
expect(json.code).toBe(2);
expect(JSON.parse(json.stdout).error.code).toBe('PACKAGE_NOT_INSTALLED');
});

it('check --json answers a refusal with its code on stdout', async () => {
const missing = await fail(
['check', 'zod', '--json'],
tempRepo({ 'package.json': manifest, 'pnpm-lock.yaml': '' }),
);
expect(missing.code).toBe(2);
expect(JSON.parse(missing.stdout)).toEqual({
error: {
code: 'PACKAGE_NOT_INSTALLED',
message:
'zod is declared in package.json but not installed: nothing is installed (no node_modules)',
},
});
const engine = fakeEngine({ declared: async () => new Map([['zod', '^3.23.8']]) });
const absent = await fail(['check', '--only', 'stripe', '--json'], npmRepo(), engine);
expect(absent.code).toBe(2);
expect(JSON.parse(absent.stdout)).toEqual({
error: { code: 'ANALYSIS_FAILED', message: 'stripe is not a dependency of shop' },
});
});

it('missing Berry installation names the immutable install command', async () => {
const { code, stderr } = await fail(
['check', 'zod', 'stripe'],
Expand Down
42 changes: 30 additions & 12 deletions packages/cli/src/failures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,23 @@ export function yarnPlugAndPlay(root: string): CliError {
});
}

export function missingNodeModules(repo: Repo, missing: readonly string[]): CliError {
/** Each missing dependency with a workspace that declares it. */
export function missingNodeModules(
repo: Repo,
missing: readonly { name: string; workspace: string }[],
): CliError {
const and = (list: readonly string[]) => names(list).replace(' or ', ' and ');
const one = missing.length === 1;
const manifests = [...new Set(missing.map((m) => join(m.workspace, 'package.json')))];
return new CliError(
`${names(missing).replace(' or ', ' and ')} ${missing.length === 1 ? 'is' : 'are'} in the lockfile but not installed in node_modules`,
`${and(missing.map((m) => m.name))} ${one ? 'is' : 'are'} declared in ${and(manifests)} but not installed: ${
existsSync(join(repo.root, 'node_modules'))
? `node_modules is there, without ${one ? 'it' : 'them'}`
: 'nothing is installed (no node_modules)'
}`,
{
why: 'The analysis compiles your code against the installed and the target version.',
code: 'PACKAGE_NOT_INSTALLED',
why: 'Install the dependencies first, then run the same command again. The analysis compiles your code against the installed and the target version.',
next:
repo.manager === 'yarn' &&
(existsSync(join(dirname(repo.lockfile), '.yarnrc.yml')) ||
Expand Down Expand Up @@ -133,21 +145,27 @@ export function networkFailure(report: CheckResult): CliError | undefined {
: undefined;
}

/** Whether `name` resolves from `dir` the way Node would look it up. */
function installedIn(dir: string, name: string): boolean {
for (let current = dir; ; current = dirname(current)) {
/**
* Whether `name` resolves from workspace `w` the way Node would look it up, up to the
* project root: a copy above it (a stray `~/node_modules`) is not this repository's install
* (UPT-136).
*/
function installedIn(repo: Repo, w: string, name: string): boolean {
const top = resolve(repo.root);
for (let current = join(top, w); ; current = dirname(current)) {
if (existsSync(join(current, 'node_modules', name, 'package.json'))) return true;
if (dirname(current) === current) return false;
if (current === top || dirname(current) === current) return false;
}
}

/** Before any analysis: the selected dependencies exist, are locked and are installed. */
export function requireInstalled(repo: Repo, dependencies: readonly DependencyStatus[]): void {
for (const dep of dependencies)
if (dep.workspaces.length > 0 && !dep.installed) throw notInLockfile(repo, dep.name);
const missing = dependencies
.filter((dep) => dep.workspaces.some((w) => !installedIn(join(repo.root, w), dep.name)))
.map((dep) => dep.name);
const missing = dependencies.flatMap((dep) => {
const workspace = dep.workspaces.find((w) => !installedIn(repo, w, dep.name));
return workspace === undefined ? [] : [{ name: dep.name, workspace }];
});
if (missing.length > 0) throw missingNodeModules(repo, missing);
}

Expand All @@ -159,12 +177,12 @@ export function requireInstalled(repo: Repo, dependencies: readonly DependencySt
export function requireNodeModules(repo: Repo, dependencies: readonly DependencyStatus[]): void {
const declared = dependencies.filter((dep) => dep.workspaces.length > 0);
const missing = declared.filter((dep) =>
dep.workspaces.every((w) => !installedIn(join(repo.root, w), dep.name)),
dep.workspaces.every((w) => !installedIn(repo, w, dep.name)),
);
if (declared.length > 0 && missing.length === declared.length)
throw missingNodeModules(
repo,
missing.slice(0, 3).map((dep) => dep.name),
missing.slice(0, 3).map((dep) => ({ name: dep.name, workspace: dep.workspaces[0] ?? '.' })),
);
}

Expand Down
30 changes: 30 additions & 0 deletions packages/core/src/fix/isolate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,36 @@ describe('preflight refusals become agent-led drafts with an agent (UPT-78)', ()
expect(result.agentFallback?.join(';')).toContain('the check returned nothing to analyze');
});

it('an empty check without an install says so, not "has no upgrade" (UPT-136)', async () => {
const { root, services } = zodFixture(scratch);
const empty = async () => ({ ...(await services.check({ cwd: root })), packages: [] });
rmSync(join(root, 'node_modules'), { recursive: true });
const error = await isolatedFix(
{ cwd: root, ...agent },
{ ...services, check: empty, agentRunner: runner() },
).catch((e: unknown) => e);
expect(error).toMatchObject({
code: 'PACKAGE_NOT_INSTALLED',
message:
'zod is declared in package.json but not installed. Install the dependencies first: pnpm install --frozen-lockfile, then run uptide fix zod again.',
});
});

it('a package already at the target says so', async () => {
const { root, services } = zodFixture(scratch);
const empty = async () => ({ ...(await services.check({ cwd: root })), packages: [] });
writeFileSync(
join(root, 'node_modules/zod/package.json'),
JSON.stringify({ name: 'zod', version: '4.6.5', types: 'index.d.ts' }),
);
await expect(
isolatedFix(
{ cwd: root, only: 'zod', fixer: null, tool: TOOL },
{ ...services, check: empty },
),
).rejects.toThrow(/^zod is already at 4\.6\.5 \(target 4\.6\.5\)$/);
});

it('never falls back on a safety refusal', async () => {
const { root, services } = zodFixture(scratch);
const planner = vi
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/fix/managers/manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { join } from 'node:path';
import { afterAll, expect, it } from 'vitest';
import { versionRange } from '../versions.js';
import { assertLockScope } from './lock-guard.js';
import { berrySkipBuild, packageManager, updateArgs } from './manager.js';
import { berrySkipBuild, frozenInstall, packageManager, updateArgs } from './manager.js';
import { pnpmGraph, yarnGraph } from './text-lock.js';

const roots: string[] = [];
Expand All @@ -18,6 +18,14 @@ function fixture(files: Record<string, string | undefined>) {
for (const [f, t] of Object.entries(files)) if (t !== undefined) writeFileSync(join(root, f), t);
return root;
}
it.each([
[{ 'package-lock.json': '{"lockfileVersion":3}' }, 'npm ci'],
[{ 'pnpm-lock.yaml': '' }, 'pnpm install --frozen-lockfile'],
[{ 'yarn.lock': '' }, 'yarn install --frozen-lockfile'],
[{ 'yarn.lock': '__metadata:\n version: 8\n' }, 'yarn install --immutable'],
])('the install a person runs for %j is %s', (files, command) => {
expect(frozenInstall(packageManager(fixture(files)))).toBe(command);
});
it.each(['^', '~', ''])('preserves the %s range style', (style) => {
expect(versionRange(`${style}3.23.8`, '4.6.5')).toBe(`${style}4.6.5`);
});
Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/fix/managers/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ export function packageManager(root: string): PackageManager {
'fix supports npm (lockfile v2/v3), pnpm, and Yarn classic/Berry; no supported lockfile found',
);
}
/** The install a person runs to get exactly what the lockfile pins. */
export function frozenInstall(pm: PackageManager): string {
return {
npm: 'npm ci',
pnpm: 'pnpm install --frozen-lockfile',
'yarn-classic': 'yarn install --frozen-lockfile',
'yarn-berry': 'yarn install --immutable',
}[pm.kind];
}
/** Manifests have already been updated, so npm can preserve dependency sections with no add/save flags. */
export function updateArgs(pm: PackageManager, root: string, workspaces: string[]): string[] {
const args = managerArgs(pm, root, workspaces);
Expand Down
Loading
Loading