From 1ba1c2a32a0a30efdf557d554cd8586b60658780 Mon Sep 17 00:00:00 2001 From: Benjamin Diedrichsen Date: Wed, 29 Jul 2026 13:00:10 +0200 Subject: [PATCH] [chore] commit id display on version --- .gitea/workflows/publish-snapshot.yml | 5 +- .gitea/workflows/release.yml | 14 ++ CLAUDE.md | 30 ++- packages/cubes-core/README.md | 6 +- packages/keyman/src/keyman.cli.ts | 14 +- packages/nopy/README.md | 8 +- packages/nopy/docs/API.md | 13 +- packages/nopy/docs/CUBE-BUNDLES.md | 38 ++-- packages/nopy/docs/CUBE-PACKAGES.md | 21 +- packages/nopy/src/cubes/packages.ts | 42 +++- packages/nopy/src/index.ts | 9 + packages/nopy/src/nopy.cli.ts | 24 ++- packages/nopy/src/nopy.exit.ts | 132 +++++++++++++ packages/nopy/src/nopy.prompts.ts | 6 +- packages/nopy/tests/cubes.packages.test.ts | 39 +++- packages/nopy/tests/exit.test.ts | 218 +++++++++++++++++++++ packages/nopy/tests/prompts.test.ts | 26 +++ 17 files changed, 589 insertions(+), 56 deletions(-) create mode 100644 packages/nopy/src/nopy.exit.ts create mode 100644 packages/nopy/tests/exit.test.ts diff --git a/.gitea/workflows/publish-snapshot.yml b/.gitea/workflows/publish-snapshot.yml index 8ad471f..78db30c 100644 --- a/.gitea/workflows/publish-snapshot.yml +++ b/.gitea/workflows/publish-snapshot.yml @@ -123,7 +123,10 @@ jobs: # `g` prefix keeps the identifier a valid semver one even when the # abbreviated sha happens to be all digits. version="${base}-main.${{ github.run_number }}.g${short_sha}" - (cd "$dir" && npm pkg set "version=${version}") + # `buildInfo.commit` is what `nopy --version` annotates itself with. + # An unknown top-level key is ignored by npm and package.json is + # always in the tarball, so it ships without any `files` change. + (cd "$dir" && npm pkg set "version=${version}" "buildInfo.commit=${short_sha}") done # Pass 2: publish. diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index c86332b..6fbdc8f 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -165,6 +165,20 @@ jobs: # Explicit, so the publish steps can skip lifecycle scripts entirely. run: pnpm run build + - name: Stamp the commit into the manifest + # What `nopy --version` annotates itself with. The version is untouched: + # this only adds a `buildInfo.commit` key, which npm ignores and which + # ships regardless of `files` because package.json is always packed. + # Before the pack below, so the artefact under test is the one publish + # ships. The tree is left dirty, which is why both publish steps pass + # --no-git-checks — they already did, for the detached HEAD. + env: + DIR: ${{ steps.target.outputs.dir }} + run: | + set -euo pipefail + short_sha=$(git rev-parse --short=7 HEAD) + (cd "$DIR" && npm pkg set "buildInfo.commit=${short_sha}") + - name: Verify the packed manifests # Packages link to each other with `workspace:*`, which npm cannot # install. Proves on the tarball that pack rewrote it. diff --git a/CLAUDE.md b/CLAUDE.md index 7a55312..f3c0c3a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,6 +22,13 @@ publish order matters — see *Releasing*. `.nopyrc.json` names it in `cubePackages`, and the loader reads it out of `node_modules`. There is no `cubes/` directory at the repo root any more. +## Documenting + +Be modest. Size the write-up to the change: most work needs none, and a small +module never earns a section in `docs/API.md`. Where a reason is genuinely +non-obvious, one comment next to the code beats three paragraphs in a document +nobody re-reads. Document the surprising, not the obvious. + ## Commands ```sh @@ -97,14 +104,19 @@ One pass per invocation, `nopy.main.ts` orchestrating: if no config file exists anywhere — which is why `nopy.cli.ts` calls it lazily inside the action, so `--help`/`--version` work outside a project. 2. **`cubes/packages.ts`** — `resolveCubePackages()` turns each `CubePackageRef` - into a package root plus the directories its `nopy.cubes` field declares. + into a package root plus its cube directories. The location is a **convention**: + `/cubes`, so a bundle needs no nopy-specific `package.json` field at all. + `nopy.cubes` survives only as an override, for the bundle whose cubes are + elsewhere (`dist/cubes` after a build, say) — absent means the default, but + present-and-malformed is an error rather than a fall back, since saying + something that does not parse is not the same as saying nothing. Resolution goes through `createRequire(...).resolve.paths()` + `existsSync`, deliberately bypassing the `exports` map: a bundle ships directories and has no entry point to declare. `existsSync` also follows the symlink pnpm plants at `node_modules/`, which a `readdir` scan skips outright (it reports `isSymbolicLink()`, not `isDirectory()`). A missing package, an unreadable - manifest, a missing `nopy.cubes`, a directory that does not exist, and an - entry pointing outside the package root are all errors, never silent skips. + manifest, no cube directory found, and an entry pointing outside the package + root are all errors, never silent skips. Duplicate refs are deduped here, last-wins, because `mergeValue` only dedupes arrays of primitives and these are objects. 3. **`cubes/loader.ts`** — `findCubeRoots()` unions `config.cubeDirs`, the @@ -295,6 +307,18 @@ numerically highest version on npmjs, so install with an explicit `@latest`. So: bump `packages//package.json`, land it on `main`, then tag that commit. +Both workflows also stamp `buildInfo.commit` (the 7-char sha) into the manifest +with the same `npm pkg set`, never committed either — the snapshot loop stamps +every package, and the release step stamps whichever one the tag named. Both +CLIs append it to `--version` in parentheses — `0.5.0 (ab12cd7)` — and print the +bare version when the field is absent, which is every run from source. The +version string itself is untouched: `nopy.cli.ts` and `keyman.cli.ts` decorate +only the string they print, while `updateNotice()` and `selfUpdate()` keep +reading the raw `version`, so channel derivation never sees the annotation. An +unknown top-level key is ignored by npm and `package.json` is always packed, so +nothing in `files` had to change. The two CLIs are kept in step here for the +same reason their update modules are duplicated rather than shared. + Three things the `workspace:*` links added, all of them non-obvious: - **`pnpm publish`, never `npm publish`.** `link-workspace-packages` is unset and diff --git a/packages/cubes-core/README.md b/packages/cubes-core/README.md index de240c9..3c731a2 100644 --- a/packages/cubes-core/README.md +++ b/packages/cubes-core/README.md @@ -18,9 +18,9 @@ Then name it in `.nopyrc.json`: } ``` -`nopy` resolves the package from the directory of the config file that named it, -reads `nopy.cubes` out of its `package.json`, and scans those directories exactly -as it scans a `cubeDirs` entry. Nothing has to be linked or copied. +`nopy` resolves the package from the directory of the config file that named it +and scans its `cubes/` directory exactly as it scans a `cubeDirs` entry. Nothing +has to be linked or copied. ## What is in it diff --git a/packages/keyman/src/keyman.cli.ts b/packages/keyman/src/keyman.cli.ts index 6a63f1b..84113a3 100644 --- a/packages/keyman/src/keyman.cli.ts +++ b/packages/keyman/src/keyman.cli.ts @@ -6,7 +6,17 @@ import { keyman } from './keyman.main.js'; import type { Channel } from './keyman.update.js'; import { formatCommand, selfUpdate, updateNotice } from './keyman.update.js'; -const { version } = createRequire(import.meta.url)('../package.json') as { version: string }; +const { version, buildInfo } = createRequire(import.meta.url)('../package.json') as { + version: string; + buildInfo?: { commit?: string }; +}; + +/** + * What `--version` prints. `version` itself stays untouched everywhere else — + * the commit is an annotation, stamped into `package.json` on the runner by the + * publish workflows and absent when running from source. + */ +const versionLabel = buildInfo?.commit ? `${version} (${buildInfo.commit})` : version; const args = process.argv.slice(2); @@ -24,7 +34,7 @@ if (args.includes('--print-config')) { } if (args.includes('--version') || args.includes('-V')) { - console.log(version); + console.log(versionLabel); process.exit(0); } diff --git a/packages/nopy/README.md b/packages/nopy/README.md index 2da2747..c985149 100644 --- a/packages/nopy/README.md +++ b/packages/nopy/README.md @@ -302,12 +302,12 @@ All three are unioned and scanned the same way. A directory is a cube when it ho #### Cube packages -A cube package is an ordinary npm package that ships cube directories and points at them from its own `package.json`: +A cube package is an ordinary npm package that ships its cubes in a `cubes/` directory at its root. That is the whole contract — no nopy-specific `package.json` field is required. A bundle whose cubes live elsewhere (compiled into `dist/cubes`, say) overrides the location: ```json { - "name": "@bitsquare/cubes-core", - "nopy": { "cubes": ["./cubes"] } + "name": "@acme/cubes-web", + "nopy": { "cubes": ["./dist/cubes"] } } ``` @@ -321,7 +321,7 @@ pnpm add -D @bitsquare/cubes-core { "cubePackages": ["@bitsquare/cubes-core"] } ``` -Naming a package is a statement that cubes are expected from it, so anything wrong is an error that aborts the run rather than a silent skip: the package is not installed, it declares no `nopy.cubes`, or an entry points at a directory that does not exist or lies outside the package. +Naming a package is a statement that cubes are expected from it, so anything wrong is an error that aborts the run rather than a silent skip: the package is not installed, it has neither a `cubes/` directory nor a `nopy.cubes` override, its `nopy.cubes` is malformed, or an entry points at a directory that does not exist or lies outside the package. #### Ids are claimed globally diff --git a/packages/nopy/docs/API.md b/packages/nopy/docs/API.md index 42ba8d7..c4e2336 100644 --- a/packages/nopy/docs/API.md +++ b/packages/nopy/docs/API.md @@ -325,8 +325,9 @@ mirror the path, and they are claimed **globally** — across `cubeDirs`, - a duplicate id (the message names every claimant and how each got into the run); - a manifest that throws on import, exports a non-object, or has no `name`; - a `secrets` entry naming a key that is not in the schema; -- a package in `cubePackages` that is not installed, cannot be read, or declares - no `nopy.cubes`; +- a package in `cubePackages` that is not installed, cannot be read, has neither a + `cubes/` directory nor a `nopy.cubes` override, or whose `nopy.cubes` is not a + non-empty array of strings; - a `nopy.cubes` entry that does not exist or points outside its package root. Any of them aborts the run (`nopy.main.ts` returns before the workflow). None is @@ -380,7 +381,7 @@ const { packages, errors } = resolveCubePackages(config.cubePackages); interface CubePackage { name: string; // the name it was requested under root: string; // absolute path to the package root - dirs: string[]; // absolute paths, from the package's `nopy.cubes` field + dirs: string[]; // absolute paths: `/cubes`, or the package's `nopy.cubes` } ``` @@ -892,8 +893,8 @@ Nothing feeds the result into the built command — see const { selectedCubes } = await CubeSelection(cubes); // string[] of ids ``` -Multi-select with fuzzy filtering on the rendered label. A cancelled prompt -returns an empty array rather than throwing. +Multi-select with fuzzy filtering on the rendered label. A prompt dismissed with +Escape returns an empty array rather than throwing. ### `HostSelection(hosts)` @@ -1182,7 +1183,7 @@ Real behaviour that a reader would otherwise take on trust. Tracked in `Cube.getDefaults()`, against `{}` — and prompt input is type-coerced, which is not the same thing. - **Nothing checks bundle/CLI compatibility.** A cube package declares no - supported nopy range and the loader reads whatever `nopy.cubes` points at. + supported nopy range and the loader scans whatever directories it finds. - **`self-update` reports an empty channel as unreachable.** `latest === null` means either the request failed *or* the registry answered normally and the dist-tag simply has no version — the second is exactly what a Gitea package diff --git a/packages/nopy/docs/CUBE-BUNDLES.md b/packages/nopy/docs/CUBE-BUNDLES.md index 0f00f33..fdfff83 100644 --- a/packages/nopy/docs/CUBE-BUNDLES.md +++ b/packages/nopy/docs/CUBE-BUNDLES.md @@ -22,13 +22,13 @@ The rest of this document is for writing one. ## What a bundle is -An ordinary npm package that ships cube directories and points at them from its -own `package.json`. There is no build step, no plugin API and no entry point — -nopy reads the directories off disk and imports each `manifest.mjs` directly. +An ordinary npm package that ships its cubes in a `cubes/` directory. There is no +build step, no plugin API and no entry point — nopy reads the directory off disk +and imports each `manifest.mjs` directly. ``` @acme/cubes-web -├── package.json nopy.cubes → ["./cubes"] +├── package.json no nopy block needed ├── README.md └── cubes/ ├── nginx/ @@ -50,7 +50,6 @@ special-cased. "name": "@acme/cubes-web", "version": "1.0.0", "type": "module", - "nopy": { "cubes": ["./cubes"] }, "files": ["cubes", "!cubes/**/*.log", "README.md", "LICENSE"], "publishConfig": { "access": "public" }, "dependencies": { @@ -60,10 +59,24 @@ special-cased. } ``` -**`nopy.cubes`** is the only field nopy requires. It is an array of directories, -relative to the package root, each scanned recursively for cubes. Several -entries are fine; a single `["./cubes"]` is the norm. Every entry must exist and -must stay inside the package — a path escaping the root is refused, not resolved. +**Nothing declares the cubes.** `cubes/` at the package root is the convention, +scanned recursively, and a bundle that follows it needs no nopy-specific field at +all. Naming the package in `cubePackages` is already the statement that cubes are +expected from it. + +**`nopy.cubes`** overrides that, for the bundle whose cubes are somewhere else — a +package compiled from TypeScript sources into `dist/cubes`, say, or one shipping +two separate trees: + +```json + "nopy": { "cubes": ["./dist/cubes", "./contrib"] } +``` + +It is an array of directories relative to the package root. Every entry must +exist and must stay inside the package — a path escaping the root is refused, not +resolved. Present-but-malformed (an empty array, a bare string, non-strings) is an +error rather than a fall back to the default: saying something that does not parse +is not the same as saying nothing. **`type: "module"`** matters: manifests are ESM. Without it a `manifest.mjs` still loads (the extension carries the day), but anything it imports relatively will @@ -222,7 +235,7 @@ Nothing bundle-specific: `npm publish` (or `pnpm publish`) with a version bump. Some things worth deciding once: - **Version the bundle independently of nopy.** There is no compatibility check - between the two — the loader reads whatever `nopy.cubes` points at. Document + between the two — the loader scans whatever directories it finds. Document the nopy version you test against in your README. - **Renaming or removing an id is breaking.** It invalidates recorded sessions and breaks any manifest listing it as a dependency, including manifests in @@ -254,8 +267,9 @@ For how this repository releases its own packages, see | Symptom | Cause | | --- | --- | | `Cube package 'X' is not installed (looked up from …)` | Not installed, or installed somewhere other than the config that named it. The path in the message is where the lookup started. | -| `Cube package 'X' declares no cubes` | Missing or malformed `nopy.cubes` in the package's `package.json`. It must be a non-empty array of strings. | -| `'./cubes' does not exist in …` | The directory was not packed. Check `files` and `npm pack --dry-run`. | +| `Cube package 'X' has no cubes/ directory in …` | No `cubes/` at the package root and no `nopy.cubes` pointing elsewhere. Usually the directory was not packed — check `files` and `npm pack --dry-run`. | +| `"nopy": { "cubes": … } must be a non-empty array of strings` | The override is present but malformed. Fix it, or omit it entirely to use `./cubes`. | +| `'…' does not exist in …` | A `nopy.cubes` entry pointing at a directory the tarball does not contain. | | `'…' points outside the package` | A `nopy.cubes` entry escaping the package root. Not allowed. | | `Duplicate cube id 'X' from N sources:` | Two or more cubes claiming one id; the message lists each source. Rename one — there is no precedence rule to lean on. | | `ERR_MODULE_NOT_FOUND` for `zod` or `@bitsquare/nopy-cube` | The bundle did not declare them as dependencies. The resolve-hook fallback covers loose local cubes, not published packages. | diff --git a/packages/nopy/docs/CUBE-PACKAGES.md b/packages/nopy/docs/CUBE-PACKAGES.md index aeeb2f9..5e92462 100644 --- a/packages/nopy/docs/CUBE-PACKAGES.md +++ b/packages/nopy/docs/CUBE-PACKAGES.md @@ -141,7 +141,6 @@ A cube bundle is an npm package with a `nopy` field: "name": "@acme/cubes-net", "version": "1.0.0", "type": "module", - "nopy": { "cubes": ["./cubes"] }, "files": ["cubes", "README.md", "LICENSE"], "keywords": ["nopy", "nopy-cubes", "pyinfra"], "dependencies": { @@ -154,10 +153,16 @@ A cube bundle is an npm package with a `nopy` field: Rules: -- `nopy.cubes` — directories relative to the package root, scanned exactly like - `cubeDirs` entries. Required; a package listed in `cubePackages` without a - `nopy` field is an error, not a silent skip. Listing it means the user expects - cubes from it. +- **Cube location is a convention, `/cubes`.** *(Amended after Phase 5; + originally `nopy.cubes` was a required field.)* The field bought nothing a + convention does not: it is not a discovery marker — naming the package in + `cubePackages` already is one — and it says nothing about whether the + directories were actually packed, which is the failure authors really hit. + `nopy.cubes` remains as an **override**, directories relative to the package + root, for the bundle whose cubes are elsewhere (`dist/cubes` after a build). + Absent means the default; present-and-malformed is an error rather than a fall + back. Finding no cube directory at all is still an error, not a silent skip: + listing a package means the user expects cubes from it. - Both dependencies are **regular dependencies, not peers**, and both are load-bearing: a manifest imports `Manifest` from `@bitsquare/nopy-cube` and `z` from `zod`. `@bitsquare/nopy-cube` peer-depends on zod, so the bundle's copy is @@ -255,7 +260,8 @@ Errors (each aborts the run, consistent with the existing `errors` contract): - package not found on any candidate path - `package.json` unparseable -- no `nopy.cubes`, or it is not a non-empty array of strings +- neither a `cubes/` directory nor a `nopy.cubes` override +- `nopy.cubes` present but not a non-empty array of strings - a `nopy.cubes` entry escapes the package root, or does not exist ### Wiring @@ -625,7 +631,8 @@ runner alike. - resolves through a symlinked package directory (mimicking pnpm) - resolves from the declaring config's directory, not `cwd` - missing package → error naming the spec -- package without `nopy.cubes` → error +- package without `nopy.cubes` → falls back to `cubes/` +- package with neither → error; malformed `nopy.cubes` → error, no fall back - `nopy.cubes` entry that does not exist, and one that escapes the root → errors - last-wins dedupe when parent and child config both name a package diff --git a/packages/nopy/src/cubes/packages.ts b/packages/nopy/src/cubes/packages.ts index f84c5e0..9ba5dc1 100644 --- a/packages/nopy/src/cubes/packages.ts +++ b/packages/nopy/src/cubes/packages.ts @@ -8,13 +8,23 @@ import { createRequire } from 'node:module'; import path from 'node:path'; import type { CubePackageRef } from '../nopy.config.js'; +/** + * Where a bundle's cubes live when its `package.json` does not say otherwise. + * + * Convention over configuration: shipping `cubes/` at the package root needs no + * `nopy` block at all. `nopy.cubes` remains as an override for the bundle whose + * cubes sit somewhere else — one compiled from TypeScript into `dist/cubes`, + * say — so the escape hatch survives without every author paying for it. + */ +const DEFAULT_CUBE_DIRS = ['./cubes']; + /** An installed cube package, located and validated. */ export interface CubePackage { /** The name it was requested under. */ name: string; /** Absolute path to the package root. */ root: string; - /** Absolute paths to its cube directories, from `nopy.cubes`. */ + /** Absolute paths to its cube directories, from `nopy.cubes` or the default. */ dirs: string[]; } @@ -40,7 +50,8 @@ function findPackageRoot(ref: CubePackageRef): string | undefined { } /** - * Resolves every named package to its cube directories. + * Resolves every named package to its cube directories — {@link DEFAULT_CUBE_DIRS} + * unless its `package.json` overrides them with `nopy.cubes`. * * Anything wrong is an error rather than a silent skip: naming a package in * `cubePackages` is a statement that cubes are expected from it, and errors @@ -75,27 +86,40 @@ export function resolveCubePackages(refs: CubePackageRef[]): { continue; } + // Absent is the ordinary case and means the convention. Present-but-wrong + // is a different thing entirely — the author meant to say something and it + // did not parse — so it stays an error rather than falling back silently. const declared = manifest.nopy?.cubes; + const defaulted = declared === undefined; + if ( - !Array.isArray(declared) || - declared.length === 0 || - !declared.every((entry) => typeof entry === 'string') + !defaulted && + (!Array.isArray(declared) || + declared.length === 0 || + !declared.every((entry) => typeof entry === 'string')) ) { errors.push( - `Cube package '${ref.spec}' declares no cubes. ` + - `Expected "nopy": { "cubes": ["./cubes"] } in ${root}/package.json.` + `Cube package '${ref.spec}': "nopy": { "cubes": … } in ${root}/package.json ` + + `must be a non-empty array of strings. Omit it to use the default, ./cubes.` ); continue; } const dirs: string[] = []; - for (const entry of declared as string[]) { + for (const entry of defaulted ? DEFAULT_CUBE_DIRS : (declared as string[])) { const dir = path.resolve(root, entry); if (dir !== root && !dir.startsWith(root + path.sep)) { errors.push(`Cube package '${ref.spec}': '${entry}' points outside the package.`); } else if (!fs.existsSync(dir)) { - errors.push(`Cube package '${ref.spec}': '${entry}' does not exist in ${root}.`); + // Naming the entry would be misleading when nobody wrote one; say what + // was looked for and what would change it instead. + errors.push( + defaulted + ? `Cube package '${ref.spec}' has no cubes/ directory in ${root}, and its ` + + `package.json declares no "nopy": { "cubes": [...] } pointing elsewhere.` + : `Cube package '${ref.spec}': '${entry}' does not exist in ${root}.` + ); } else { dirs.push(dir); } diff --git a/packages/nopy/src/index.ts b/packages/nopy/src/index.ts index ed510ff..16ade49 100644 --- a/packages/nopy/src/index.ts +++ b/packages/nopy/src/index.ts @@ -36,6 +36,15 @@ export { outputExecutionPlan, summarizeResults, } from './nopy.executor.js'; +// Graceful exit +export { + CANCELLED_EXIT_CODE, + exitWithFarewell, + FAREWELL, + installGracefulExit, + isCancellation, + restoreTerminal, +} from './nopy.exit.js'; export type { HistoryEntry, SessionHistory } from './nopy.history.js'; // History management export { diff --git a/packages/nopy/src/nopy.cli.ts b/packages/nopy/src/nopy.cli.ts index 4bad1f5..7be9bdc 100644 --- a/packages/nopy/src/nopy.cli.ts +++ b/packages/nopy/src/nopy.cli.ts @@ -8,6 +8,7 @@ import { createRequire } from 'node:module'; import { Command } from 'commander'; import { loadConfig } from './nopy.config.js'; +import { exitWithFarewell, installGracefulExit, isCancellation } from './nopy.exit.js'; import { clearHistory, formatHistoryList, @@ -19,7 +20,17 @@ import { nopy } from './nopy.main.js'; import type { Channel } from './nopy.update.js'; import { formatCommand, selfUpdate, updateNotice } from './nopy.update.js'; -const { version } = createRequire(import.meta.url)('../package.json') as { version: string }; +const { version, buildInfo } = createRequire(import.meta.url)('../package.json') as { + version: string; + buildInfo?: { commit?: string }; +}; + +/** + * What `--version` prints. `version` itself stays untouched everywhere else — + * the commit is an annotation, stamped into `package.json` on the runner by the + * publish workflows and absent when running from source. + */ +const versionLabel = buildInfo?.commit ? `${version} (${buildInfo.commit})` : version; /** * Prints the update hint to stderr, so it never lands in `--json` output or in @@ -32,11 +43,15 @@ async function printUpdateNotice(): Promise { } } +// Before anything can open a prompt: a cancelled TUI leaves through +// nopy.exit, not through node's default unhandled-rejection trace. +installGracefulExit(); + const program = new Command(); program .name('nopy') - .version(version) + .version(versionLabel) .description('A CLI tool for pyinfra script management and execution.') .addHelpText( 'after', @@ -124,6 +139,11 @@ program process.exit(1); } } catch (error) { + // A prompt the user backed out of is not a failed run: inquirer rejects + // cleanly, so unlike the enquirer case this arrives here rather than at + // the process-level handler. + if (isCancellation(error)) exitWithFarewell(); + if (options.json) { console.log( JSON.stringify( diff --git a/packages/nopy/src/nopy.exit.ts b/packages/nopy/src/nopy.exit.ts new file mode 100644 index 0000000..eec3009 --- /dev/null +++ b/packages/nopy/src/nopy.exit.ts @@ -0,0 +1,132 @@ +/** + * What happens when the user walks out of the TUI instead of finishing it. + * @module nopy.exit + */ + +/** Parting words. Printed whenever a run ends because the user asked it to. */ +export const FAREWELL = 'Bye Bye Honeypie'; + +/** Conventional exit code for "terminated by SIGINT" — 128 + 2. */ +export const CANCELLED_EXIT_CODE = 130; + +/** ETX: the byte a raw-mode terminal delivers for Ctrl-C. */ +const ETX = '\x03'; + +/** Undoes `ansi.cursor.hide()`, which every enquirer prompt writes on start. */ +const SHOW_CURSOR = '\x1B[?25h'; + +/** + * Error names the two prompt libraries use for "the user called it off". + * + * `ExitPromptError` is what `@inquirer/core` rejects with on Ctrl-C; + * `CancelPromptError` is the same thing reached from outside the prompt. + */ +const CANCEL_ERROR_NAMES = new Set(['ExitPromptError', 'CancelPromptError']); + +/** + * Whether a thrown value is the user cancelling rather than something failing. + * + * Three shapes, one per way out of a prompt: + * + * - `ERR_USE_AFTER_CLOSE` — enquirer's teardown exploding. Ctrl-C in raw mode + * reaches *both* node's readline, which closes the interface because it has + * no `SIGINT` listener, and enquirer's own keypress queue, which then cancels + * the prompt and calls `rl.pause()` on the interface node has already closed. + * Node >= 22 throws there rather than ignoring it. The throw happens inside + * `Prompt.close()`, i.e. *before* `emit('cancel')`, so `prompt.run()` never + * settles and the `try/catch` around it in `nopy.prompts` never runs — the + * rejection surfaces with nothing awaiting it, which is why this has to be + * caught at the process level. + * - `ExitPromptError` — inquirer, which does reject cleanly and whose rejection + * travels up the normal call chain. + * - a bare `''` or an ETX byte — enquirer rejecting a cancelled prompt with the + * keypress that cancelled it, on the runs where the teardown does not throw. + */ +export function isCancellation(error: unknown): boolean { + if (error === '' || error === ETX) return true; + if (typeof error !== 'object' || error === null) return false; + + const { name, code } = error as { name?: unknown; code?: unknown }; + return ( + code === 'ERR_USE_AFTER_CLOSE' || (typeof name === 'string' && CANCEL_ERROR_NAMES.has(name)) + ); +} + +/** + * Puts the terminal back the way it was found. + * + * A prompt owns the terminal while it runs: stdin is in raw mode and the cursor + * is hidden. Exiting from under it leaves the shell with no cursor and no echo, + * so this runs on every abnormal exit, cancelled or crashed. Best-effort by + * design — a destroyed stdin throws on `setRawMode`, and a failure to tidy up + * must not replace the message explaining why we are leaving. + */ +export function restoreTerminal(): void { + try { + if (process.stdin.isTTY && process.stdin.isRaw) process.stdin.setRawMode(false); + if (process.stdout.isTTY) process.stdout.write(SHOW_CURSOR); + } catch { + // Nothing useful to do about a terminal that will not be restored. + } +} + +/** + * Says goodbye and leaves. + * + * The farewell goes to **stderr**, for the same reason the update hint does: + * `--json` and `--print-only` stay machine-readable no matter how the run ends. + * + * `process.exit` rather than letting the loop drain, because the prompt that + * was cancelled is still holding stdin — after the teardown above threw, its + * promise is pending forever and nothing else will end the process. + */ +export function exitWithFarewell(code: number = CANCELLED_EXIT_CODE): never { + restoreTerminal(); + process.stderr.write(`\n${FAREWELL}\n`); + return process.exit(code) as never; +} + +/** + * Reports a genuine crash, having first handed the terminal back. + * + * Deliberately as loud as node's own default — the stack, not a summary. The + * only thing being taken over is *when* it prints, so that {@link + * restoreTerminal} gets to run first. + */ +function reportFatal(error: unknown): never { + restoreTerminal(); + console.error(error instanceof Error ? (error.stack ?? error.message) : String(error)); + return process.exit(1) as never; +} + +/** + * Installs the process-level handlers that turn a Ctrl-C into {@link FAREWELL}. + * + * Two entry points, because Ctrl-C arrives differently depending on who owns + * the terminal. During a prompt, stdin is in raw mode: the process gets no + * `SIGINT` at all, the keypress goes to the prompt library, and the failure + * comes back as an unhandled rejection. Everywhere else — cube loading, a + * pyinfra run — the signal arrives normally. + * + * Returns a disposer, which the CLI ignores and the tests do not. + */ +export function installGracefulExit(): () => void { + const onSignal = () => exitWithFarewell(); + const onFatal = (reason: unknown) => { + if (isCancellation(reason)) { + exitWithFarewell(); + return; + } + reportFatal(reason); + }; + + process.on('SIGINT', onSignal); + process.on('uncaughtException', onFatal); + process.on('unhandledRejection', onFatal); + + return () => { + process.off('SIGINT', onSignal); + process.off('uncaughtException', onFatal); + process.off('unhandledRejection', onFatal); + }; +} diff --git a/packages/nopy/src/nopy.prompts.ts b/packages/nopy/src/nopy.prompts.ts index fd2c04a..2cd0519 100644 --- a/packages/nopy/src/nopy.prompts.ts +++ b/packages/nopy/src/nopy.prompts.ts @@ -83,7 +83,9 @@ export async function AuthSelection(useAuthKey?: boolean): Promise<{ if (useAuthKey) return { authMethod: 'ssh-key' }; const answers = await inquirer.prompt([ { - type: 'list', + // `select`, not `list`: inquirer 14 dropped the legacy name and rejects + // an unknown type outright. + type: 'select', name: 'authMethod', message: 'Select authentication method:', choices: ['ssh-key', 'password'], @@ -118,7 +120,7 @@ export async function PasswordSelection(username: string): Promise { export async function HostSelection(hosts: string[]): Promise { const selectedHost = await inquirer.prompt([ { - type: 'list', + type: 'select', name: 'host', message: 'Select host from inventory', choices: ['docker', 'vagrant', ...hosts, 'custom'], diff --git a/packages/nopy/tests/cubes.packages.test.ts b/packages/nopy/tests/cubes.packages.test.ts index 6fecf2b..f4d36b0 100644 --- a/packages/nopy/tests/cubes.packages.test.ts +++ b/packages/nopy/tests/cubes.packages.test.ts @@ -108,19 +108,48 @@ describe('resolveCubePackages', () => { expect(errors[0]).toMatch(/cannot read/); }); - it('reports a package that declares no cubes', () => { - install(tmpDir, 'plain', {}); + it('falls back to cubes/ when the package declares nothing', () => { + // The convention. A bundle that ships cubes/ at its root needs no `nopy` + // block at all, and one with an unrelated `nopy` block still gets it. + const plain = install(tmpDir, 'plain', {}); + const other = install(tmpDir, 'other', { nopy: { somethingElse: true } }); + + const { packages, errors } = resolveCubePackages( + ['plain', 'other'].map((spec) => ({ spec, from: tmpDir })) + ); + + expect(errors).toEqual([]); + expect(packages.map((pkg) => pkg.dirs)).toEqual([ + [path.join(plain, 'cubes')], + [path.join(other, 'cubes')], + ]); + }); + + it('reports a package with neither a declaration nor a cubes/ directory', () => { + install(tmpDir, 'bare', {}, []); + + const { packages, errors } = resolveCubePackages([{ spec: 'bare', from: tmpDir }]); + + expect(packages).toEqual([]); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatch(/has no cubes\/ directory/); + expect(errors[0]).toMatch(/declares no "nopy"/); + }); + + it('reports a malformed declaration instead of falling back to the default', () => { + // Each of these ships a usable cubes/ directory. Saying something that does + // not parse is not the same as saying nothing, so none of them resolve. install(tmpDir, 'empty', { nopy: { cubes: [] } }); install(tmpDir, 'wrong-type', { nopy: { cubes: 'cubes' } }); install(tmpDir, 'not-strings', { nopy: { cubes: [1] } }); const { packages, errors } = resolveCubePackages( - ['plain', 'empty', 'wrong-type', 'not-strings'].map((spec) => ({ spec, from: tmpDir })) + ['empty', 'wrong-type', 'not-strings'].map((spec) => ({ spec, from: tmpDir })) ); expect(packages).toEqual([]); - expect(errors).toHaveLength(4); - for (const error of errors) expect(error).toMatch(/declares no cubes/); + expect(errors).toHaveLength(3); + for (const error of errors) expect(error).toMatch(/must be a non-empty array of strings/); }); it('reports a cube directory that does not exist', () => { diff --git a/packages/nopy/tests/exit.test.ts b/packages/nopy/tests/exit.test.ts new file mode 100644 index 0000000..e45dd14 --- /dev/null +++ b/packages/nopy/tests/exit.test.ts @@ -0,0 +1,218 @@ +/** + * Tests for nopy.exit. + * + * The handlers are invoked by calling the listener `installGracefulExit` + * registered, not by `process.emit()`-ing the event: vitest listens for + * `unhandledRejection` and `uncaughtException` itself and would report a + * synthetic one as a failure of the test file. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + CANCELLED_EXIT_CODE, + exitWithFarewell, + FAREWELL, + installGracefulExit, + isCancellation, + restoreTerminal, +} from '../src/nopy.exit.js'; + +let exit: ReturnType; +let stderr: ReturnType; + +/** Pretends stdin/stdout are the terminal a prompt would have taken over. */ +function fakeTerminal(opts: { isTTY: boolean; isRaw?: boolean }) { + const setRawMode = vi.fn(); + const original = { + isTTY: process.stdin.isTTY, + isRaw: process.stdin.isRaw, + setRawMode: process.stdin.setRawMode, + outTTY: process.stdout.isTTY, + }; + + Object.defineProperty(process.stdin, 'isTTY', { value: opts.isTTY, configurable: true }); + Object.defineProperty(process.stdin, 'isRaw', { value: opts.isRaw ?? false, configurable: true }); + Object.defineProperty(process.stdin, 'setRawMode', { value: setRawMode, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: opts.isTTY, configurable: true }); + + const restore = () => { + Object.defineProperty(process.stdin, 'isTTY', { value: original.isTTY, configurable: true }); + Object.defineProperty(process.stdin, 'isRaw', { value: original.isRaw, configurable: true }); + Object.defineProperty(process.stdin, 'setRawMode', { + value: original.setRawMode, + configurable: true, + }); + Object.defineProperty(process.stdout, 'isTTY', { value: original.outTTY, configurable: true }); + }; + + return { setRawMode, restore }; +} + +/** The listener `installGracefulExit` most recently added for `event`. */ +const lastListener = (event: string) => + process.listeners(event as 'SIGINT').at(-1) as (reason?: unknown) => void; + +beforeEach(() => { + exit = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('isCancellation', () => { + it('recognises enquirer tearing down a readline node already closed', () => { + const err = Object.assign(new Error('readline was closed'), { code: 'ERR_USE_AFTER_CLOSE' }); + + expect(isCancellation(err)).toBe(true); + }); + + it('recognises the inquirer cancellations', () => { + const exitPrompt = Object.assign(new Error('User force closed the prompt'), { + name: 'ExitPromptError', + }); + const cancelPrompt = Object.assign(new Error('Prompt was canceled'), { + name: 'CancelPromptError', + }); + + expect(isCancellation(exitPrompt)).toBe(true); + expect(isCancellation(cancelPrompt)).toBe(true); + }); + + it('recognises the bare values enquirer rejects a cancelled prompt with', () => { + expect(isCancellation('')).toBe(true); + expect(isCancellation('\x03')).toBe(true); + }); + + it('leaves a genuine failure alone', () => { + expect(isCancellation(new Error('pyinfra exited 1'))).toBe(false); + expect(isCancellation(Object.assign(new Error('nope'), { code: 'ENOENT' }))).toBe(false); + expect(isCancellation('boom')).toBe(false); + expect(isCancellation(undefined)).toBe(false); + expect(isCancellation(null)).toBe(false); + expect(isCancellation(7)).toBe(false); + }); +}); + +describe('restoreTerminal', () => { + it('leaves raw mode and shows the cursor again', () => { + const terminal = fakeTerminal({ isTTY: true, isRaw: true }); + + restoreTerminal(); + + expect(terminal.setRawMode).toHaveBeenCalledWith(false); + expect(process.stdout.write).toHaveBeenCalledWith('\x1B[?25h'); + terminal.restore(); + }); + + it('touches nothing when the output is not a terminal', () => { + const terminal = fakeTerminal({ isTTY: false }); + + restoreTerminal(); + + expect(terminal.setRawMode).not.toHaveBeenCalled(); + expect(process.stdout.write).not.toHaveBeenCalled(); + terminal.restore(); + }); + + it('survives a stdin that refuses to leave raw mode', () => { + const terminal = fakeTerminal({ isTTY: true, isRaw: true }); + terminal.setRawMode.mockImplementation(() => { + throw new Error('stdin destroyed'); + }); + + expect(() => restoreTerminal()).not.toThrow(); + terminal.restore(); + }); +}); + +describe('exitWithFarewell', () => { + it('says goodbye on stderr and exits 130', () => { + exitWithFarewell(); + + expect(stderr).toHaveBeenCalledWith(`\n${FAREWELL}\n`); + expect(exit).toHaveBeenCalledWith(CANCELLED_EXIT_CODE); + }); + + it('accepts a different exit code', () => { + exitWithFarewell(0); + + expect(exit).toHaveBeenCalledWith(0); + }); +}); + +describe('installGracefulExit', () => { + let dispose: () => void; + + beforeEach(() => { + dispose = installGracefulExit(); + }); + + afterEach(() => { + dispose(); + }); + + it('says goodbye on SIGINT', () => { + lastListener('SIGINT')(); + + expect(stderr).toHaveBeenCalledWith(`\n${FAREWELL}\n`); + expect(exit).toHaveBeenCalledWith(CANCELLED_EXIT_CODE); + }); + + it('says goodbye for the rejection nothing is awaiting', () => { + // The shape of the real crash: enquirer's Ctrl-C teardown, which leaves + // `prompt.run()` pending forever, so no `catch` in nopy.prompts sees it. + lastListener('unhandledRejection')( + Object.assign(new Error('readline was closed'), { code: 'ERR_USE_AFTER_CLOSE' }) + ); + + expect(stderr).toHaveBeenCalledWith(`\n${FAREWELL}\n`); + expect(exit).toHaveBeenCalledWith(CANCELLED_EXIT_CODE); + }); + + it('still reports a real crash, loudly, and exits 1', () => { + const boom = new Error('everything is on fire'); + + lastListener('uncaughtException')(boom); + + expect(stderr).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledWith(boom.stack); + expect(exit).toHaveBeenCalledWith(1); + }); + + it('reports a thrown non-error too', () => { + lastListener('uncaughtException')('just a string'); + + expect(console.error).toHaveBeenCalledWith('just a string'); + expect(exit).toHaveBeenCalledWith(1); + }); + + it('reports an error with no stack by its message', () => { + const stackless = new Error('no stack here'); + stackless.stack = undefined; + + lastListener('uncaughtException')(stackless); + + expect(console.error).toHaveBeenCalledWith('no stack here'); + }); + + it('hands the process back on dispose', () => { + const before = { + SIGINT: process.listenerCount('SIGINT'), + uncaughtException: process.listenerCount('uncaughtException'), + unhandledRejection: process.listenerCount('unhandledRejection'), + }; + + dispose(); + + expect(process.listenerCount('SIGINT')).toBe(before.SIGINT - 1); + expect(process.listenerCount('uncaughtException')).toBe(before.uncaughtException - 1); + expect(process.listenerCount('unhandledRejection')).toBe(before.unhandledRejection - 1); + + // The afterEach disposer runs a second time; make it a no-op. + dispose = () => {}; + }); +}); diff --git a/packages/nopy/tests/prompts.test.ts b/packages/nopy/tests/prompts.test.ts index 7d707ae..1dc664f 100644 --- a/packages/nopy/tests/prompts.test.ts +++ b/packages/nopy/tests/prompts.test.ts @@ -218,6 +218,32 @@ describe('HostSelection', () => { }); }); +describe('question types', () => { + it('declares only types the installed inquirer actually ships', async () => { + // Checked against the real module, not the mock: `list` was accepted for + // years and inquirer 14 dropped it, which took out host selection entirely + // — a failure no amount of mocked prompting can see. + const actual = await vi.importActual('inquirer'); + const supported = Object.keys(actual.createPromptModule().prompts); + + inquirerPrompt.mockResolvedValue({ host: 'web-1' }); + await HostSelection(['web-1']); + inquirerPrompt.mockResolvedValue({ authMethod: 'password', username: 'u', password: 'p' }); + await AuthSelection(false); + inquirerPrompt.mockResolvedValue({ password: 'p' }); + await PasswordSelection('deploy'); + + const declared = new Set( + inquirerPrompt.mock.calls.flatMap(([asked]: [Record[]]) => + asked.map((q) => q.type ?? 'input') + ) + ); + + expect(declared.size).toBeGreaterThan(0); + expect(supported).toEqual(expect.arrayContaining([...declared])); + }); +}); + describe('VariableAssignment', () => { const schema = z.object({ port: z.number().default(8080).describe('Listen port'),