From 05f2d6aa56d7759339cd831fe303967422b312c7 Mon Sep 17 00:00:00 2001 From: Benjamin Diedrichsen Date: Tue, 1 Sep 2026 12:49:30 +0200 Subject: [PATCH] [docs] nopy: record the seven findings this branch closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation half of the same work: `DOCS-AUDIT.md` marks §1.3, §1.5, §2.3, §4.2 (all three points), §5.1, §6.1, §6.2 and §6.5 closed, each keeping its original text as the record with what closed it quoted underneath, and the "suggested order of attack" is rewritten to what is actually left — §5.2, §5.3, the two missing cube READMEs, and the two findings (§2.7, §4.4) that are stated accurately in `docs/API.md` while the code still behaves as they describe. `docs/API.md` drops the two entries from its *Known gaps* list that are no longer gaps, documents the argv and the absent shell, describes the resolution stack and the error it raises, and inverts the `.default()`/`.describe()` warning: the order used to matter and no longer does, which is worth saying outright since the old advice is in the reader's memory and in 15 manifests. The README's "topological sorting" becomes "in dependency order, with cycle detection" — the sort never existed, but until this branch neither did the thing a sort would have been for — and `--no-history` is spelled `--no-save-history` wherever it appears. One line of code rides along, because it is what a `docs/API.md` note has been asking for: `CubePackageRef` is re-exported from `src/index.ts`, so importing `NopyConfig` from `@bitsquare/nopy` no longer gives you a type whose own members you cannot name. The note in `docs/API.md` saying it is missing goes with it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DCzYTAm9QUhvLNr2EpdagJ --- DOCS-AUDIT.md | 204 +++++++++++++++++++++++++++++-------- packages/nopy/README.md | 33 +++--- packages/nopy/docs/API.md | 81 +++++++++------ packages/nopy/src/index.ts | 1 + 4 files changed, 225 insertions(+), 94 deletions(-) diff --git a/DOCS-AUDIT.md b/DOCS-AUDIT.md index 9d0f279..7209521 100644 --- a/DOCS-AUDIT.md +++ b/DOCS-AUDIT.md @@ -15,20 +15,23 @@ Verified against the working tree at commit `fcc1817`. Line numbers are from tha state. Findings closed since are marked **✅ … fixed** and keep their original text as -the record of what was wrong. So far: §1.1 (`--use-defaults`), §2.2 -(`getDefaults()`), §2.1 (precedence — the second half closed differently than -proposed), §3 in full (`docs/API.md`, regenerated), §4.2 (password on stdout — -points 1 and 2 of 3), §4.3 (what a session records), §2.9 (the nopy README's +the record of what was wrong. So far: §1.1 (`--use-defaults`), §1.3 (`log.*`), +§1.5 (topological order, both halves), §2.2 (`getDefaults()`), §2.1 (precedence — +the second half closed differently than proposed), §2.3 (the prompt label lost to +`.default()`), §3 in full (`docs/API.md`, regenerated), §4.2 (password on stdout, +all three points), §4.3 (what a session records), §2.9 (the nopy README's yarn install instructions), §2.4 (`version` / `timestamp`, implemented rather than deleted), §2.5 (`listSessions`' filename filter), §4.5 (`-s` on a replay, -plus `-l` and history), §6.7 (a secret written to the session in plaintext), and -one bullet of §6.4. +plus `-l` and history), §5.1 and §6.1 (`service/autostart`, the README and the +script), §6.2 (`-H ` versus `--no-history`), §6.5 (cycle detection), §6.7 (a +secret written to the session in plaintext), and one bullet of §6.4. Closing §3 also settled the documentation half of several findings elsewhere -without touching their underlying cause: §1.3, §1.5, §2.3, §2.7, §4.4 and -§6.5 are each now stated accurately in `docs/API.md`, but the code still behaves -as those findings describe and they stay open. §1.2 was closed outright by -removing the flag. +without touching their underlying cause. Two of those are still in that state: +§2.7 and §4.4 are stated accurately in `docs/API.md`, but the code still behaves +as they describe and they stay open. The other four — §1.3, §1.5, §2.3 and §6.5 — +have since been closed in the code as well. §1.2 was closed outright by removing +the flag. ## Where the drift is @@ -139,7 +142,19 @@ pyinfra's own output, everything nopy says about itself goes to stderr**, and th exit code is the verdict. `nopy history --json` is a different flag, it works, and it stays. -### 1.3 🟠 `log.verbosity` and `log.debug` have no effect +### 1.3 ✅ `log.verbosity` and `log.debug` have no effect — **fixed** + +> **Closed by implementing it, not by deleting the documentation.** The two +> tables in the README were accurate about pyinfra's flags and about the mapping +> `logConfigToFlags()` already computed — the only missing step was one call. +> `buildDeployCall` now prefixes `logConfigToFlags(this.config.log)` onto the +> pyinfra argv, right after `-y`. Verified against a real `pyinfra` binary, which +> accepts `-vv --debug` in that position. +> +> Worth knowing rather than discovering: `packages/nopy/.nopyrc.json` asks for +> `"verbosity": "trace", "debug": true`, and it now gets `-vvv --debug`. The +> value was left alone — it says what its author meant, and it never did anything +> until now. Pre-existing known drift, recorded in `CLAUDE.md`, but the README still presents it as a working feature — two tables, a recommendation paragraph, and a slot in @@ -159,7 +174,25 @@ This is live in the repo's own config: `packages/nopy/.nopyrc.json` sets the current interface at `cubes/types.ts:43-56`, which has `id`, `name`, `schema`, `dependencies`, `before`, `after` and nothing else. -### 1.5 🟠 Topological sorting +### 1.5 ✅ Topological sorting — **closed, both halves** + +> **The vocabulary complaint was answered by §6.9; the substantive one is now +> fixed in code.** There is still no sort pass, and there does not need to be: +> emission is post-order, so the output *is* a topological order of the graph +> (§6.9 records the measurement and two tests pin it). What the finding was right +> about is that a sort detects cycles and this did not. +> +> `BuildContext` now carries a **resolution stack** alongside `resolvedCubes`: a +> (cube, host) pair re-entered while it is still resolving raises a +> `NopyUsageError` naming the whole path — `Circular dependency on host1: +> a → b → c → a`. It has to be a separate structure. `resolvedCubes` is written +> *after* the descent, so a cycle never reaches it, and it cannot be widened into +> a "seen" set because re-entering a *finished* cube with different `param` +> overrides is precisely what a dependency or a hook is for. Six tests cover it, +> including a loop closed by a hook's `exec` rather than a `dependencies()` +> entry, and a diamond that must still be allowed. +> +> The README claims are reworded: "in dependency order, with cycle detection". `README.md:11` ("Dependency resolution with **topological sorting**"), `README.md:25` ("Topologically sorts cubes based on dependencies") and @@ -277,7 +310,20 @@ Three cubes in this repo are in that state today: The failure is silent — no error, no warning, just a pyinfra run with an empty data set. -### 2.3 🔴 `.describe()` before `.default()` loses the prompt label +### 2.3 ✅ `.describe()` before `.default()` loses the prompt label — **fixed** + +> **Closed the one-line way, not by re-ordering 15 manifests.** `nopy.prompts.ts` +> has a `promptLabel()` that walks down through `default` / `optional` / +> `nullable` wrappers looking for a description, so both chaining orders now give +> the sentence and neither can regress. Discriminates on `zodKind`, not +> `instanceof`, for the reason recorded on that function. +> +> Proven twice over. The mocked test asserts all four shapes — +> `describe().default()`, `default().describe()`, a doubly-wrapped +> `describe().optional().default()`, and a field with no description at all. The +> **pty** test is the one that matters: its probe schema is written in the losing +> order and it now waits for `First value` on a real enquirer render, so removing +> the unwrapping fails a test that talks to an actual terminal. `CLAUDE.md` and the cube contract state that each schema field is `.describe()`d and "the description is the prompt label". `nopy.prompts.ts:184-185` reads it as: @@ -613,9 +659,30 @@ Because `loadCubes` turns each failure into an `errors` entry and `nopy.main.ts: aborts when `errors.length > 0`, a fresh clone cannot run a single cube. Neither README mentions a setup step. -### 4.2 🟠 The SSH password is printed in plaintext — **mostly fixed** +### 4.2 ✅ The SSH password is printed in plaintext — **fixed as far as it can be** -> **Points 1 and 2 resolved; point 3 stands.** `maskCommand()` +> **Point 3 is now closed too, and it was worse than the finding said.** The +> command is no longer a shell string. `buildDeployCall` emits a true argv — one +> element per argument, nothing pre-quoted — and `executeCall` spawns it as +> `execa(command[0], command.slice(1))` with no `shell` option at all. +> +> The finding called the quoting a vulnerability "in the password". It was not +> limited to the password: with `shell: true` the whole joined string was parsed +> by `sh`, and `--data` values were interpolated inside double quotes, so a `$(…)` +> or a backtick in *any* variable value was command substitution. Verified both +> ways against a real pyinfra: `--data 'MOTD=$(id); rm -rf /'` now arrives at +> `host.data.MOTD` verbatim. +> +> `maskCommand()` was rewritten to walk the argv by position rather than to +> pattern-match a joined string, which also fixes a leak the old version had — it +> bounded a secret's value on the closing `"` the builder had written, so a value +> containing a `"` leaked its own tail. It shell-quotes as it joins, so +> `--print-only` output is still pasteable. +> +> What remains is not fixable here: the value still reaches pyinfra on its +> command line and so is visible in `ps`. That is pyinfra's `--data` interface. + +> **Points 1 and 2 resolved earlier.** `maskCommand()` > (`nopy.executor.ts`) rewrites the SSH `--password` and every `--data` value the > manifest declared a secret, and it is applied at all three places the command > string is printed: the debug log, the dry-run plan, and `--print-only`. The @@ -623,10 +690,8 @@ README mentions a setup step. > says which keys are sensitive, so `TOKEN`, `PSK` and `AUTH_KEY` are covered > too, and it no longer matters that a key merely *looks* like a password. > -> Point 3 is unchanged and now documented instead: the value still reaches -> pyinfra on its command line, so it is visible in `ps`. That is inherent to -> pyinfra's `--data` interface, not something nopy can mask. The shell-quoting -> concern in the same point is also still open. See `docs/REFACTORING.md` item 7. +> (At the time: point 3 unchanged, documented rather than fixed. See +> `docs/REFACTORING.md` item 7.) Not stated in any document, and it sits directly against the security notes at `README.md:217` and `:325` (which are narrowly about *storage*, and are correct @@ -730,7 +795,15 @@ a project without `KEY_DIR` in their config gets `None`. Two cubes have **no README at all**: `cubes/admin/hostname` and `cubes/git/clone` (20 of 22 have one). -### 5.1 🔴 `cubes/service/autostart/README.md` documents a different cube +### 5.1 ✅ `cubes/service/autostart/README.md` documents a different cube — **fixed** + +> **Rewritten from the manifest and the (now working, see §6.1) deploy script.** +> Three parameters, `APP` / `SERVICE_NAME` / `AUTOSTART`, each described as what +> it actually does — including that `SERVICE_NAME` never reaches systemd and is +> a label only, so getting it wrong is cosmetic rather than a cube that manages +> the wrong unit. The new text also states the thing the old one obscured by +> describing a deploy pipeline: this cube does **not** create the unit file, it +> only enables and starts one that already exists. The file is titled **"TypeStack Install Cube"** and describes cloning a git repository, `yarn install`, `yarn build`, `docker compose up -d`, and PM2 process @@ -794,7 +867,14 @@ have empty schemas.) Not documentation issues, but found while checking the docs and worth recording. -### 6.1 🔴 `cubes/service/autostart/deploy.py` cannot run +### 6.1 ✅ `cubes/service/autostart/deploy.py` cannot run — **fixed** + +> **Three lines.** `server` is imported alongside `systemd`, and `SERVICE_NAME` +> and `AUTOSTART` are read off `host.data` next to `APP`. The logic underneath +> was always right; nothing else changed. `python3 -m py_compile` passes. +> +> The "fails twice over" clause is stale: §2.2 closed with `-D`, so the cube does +> get its `--data` now. ```python from pyinfra.operations import systemd # `server` is never imported @@ -811,7 +891,31 @@ if AUTOSTART: # NameError `host.data`; `server` is used but not imported. The script raises `NameError` on the `if`. Per §2.2 this cube also gets no `--data` at all, so it fails twice over. -### 6.2 🔴 `-H ` and `--no-history` share one destination +### 6.2 ✅ `-H ` and `--no-history` share one destination — **fixed** + +> **The boolean was renamed, not the replay flag.** `--no-history` is now +> `--no-save-history`, writing to `options.saveHistory`; `-H, --history ` +> keeps `options.history` and every documented invocation of it is unchanged. +> Renaming the boolean is the right way round twice over: `-H ` is what the +> help text, the README and `docs/API.md` all use, and "save history" is what the +> flag actually suppresses, next to `-s, --save-session`. +> +> Commander cannot be told to use a different destination — `attributeName()` is +> derived from the long flag with `no-` stripped — so separating the two meant +> changing one of the two spellings. The old spelling now fails loudly rather +> than silently, which is the point: `nopy install --no-history` prints +> `error: unknown option '--no-history'`. +> +> Verified by running the CLI, since `nopy.cli.ts` is argv wiring and excluded +> from coverage: +> +> ``` +> install -H nonexistent-id -> Session not found: nonexistent-id +> install -H nonexistent-id --no-save-history -> Session not found: nonexistent-id +> install --no-history -> error: unknown option '--no-history' +> ``` +> +> The middle line is the finding: the id used to be destroyed there. Both options write to `options.history` (`nopy.cli.ts:57` and `:64`). Verified with Commander: @@ -846,10 +950,14 @@ the three has to give. with a third one nobody had noticed: a `console.log` *inside* a `filter` callback in `keyman.decrypt.ts`, printing a line per vault directory. -### 6.5 🟡 No cycle detection +### 6.5 ✅ No cycle detection — **fixed** -Covered under §1.5. `docs/API.md:160` documents the error; there is no code that -raises it. Mutually dependent cubes recurse until the stack overflows. +Covered under §1.5, and closed there: the resolution stack raises a +`NopyUsageError` naming the whole path. `docs/API.md` documents the error again, +and this time something raises it. + +~~`docs/API.md:160` documents the error; there is no code that raises it. +Mutually dependent cubes recurse until the stack overflows.~~ ### 6.6 🟠 `ssh:keygen` depends on `user:add` but shares nothing with it @@ -998,29 +1106,37 @@ Recording what was verified and found correct, so a future pass need not redo it ## Suggested order of attack -**1 — ~~Decide on the three phantom features.~~ Two left.** §1.1 (`-D`) is -**done** — implemented, tested, and verified against every cube in `cubes/`. -That closed §2.2 and half of §2.1 with it, since neither could be left standing -under a run that never prompts. §1.2 (`--json`) is **done** — removed rather -than implemented, for the reason recorded there. §1.3 (`log.*`) is still -"documented, wired up, never read": a small implementation or a small deletion, -but it cannot stay documented as working. +**1 — ~~Decide on the three phantom features.~~ Done, three different ways.** +§1.1 (`-D`) was implemented, tested, and verified against every cube in `cubes/`; +that closed §2.2 and half of §2.1 with it, since neither could be left standing +under a run that never prompts. §1.2 (`--json`) was removed rather than +implemented, for the reason recorded there. §1.3 (`log.*`) was implemented — +`logConfigToFlags()` finally has a caller, in `buildDeployCall`. -**4 — Decide the `.describe()`/`.default()` ordering (§2.3).** Either read -through the `ZodDefault` wrapper in `nopy.prompts.ts`, or fix the ordering in all -14 manifests and the README example. The first is one line and cannot regress. +**4 — ~~Decide the `.describe()`/`.default()` ordering (§2.3).~~ Done, by +reading through the wrapper.** `promptLabel()` in `nopy.prompts.ts` walks +`default`/`optional`/`nullable` down to the described schema, so both orders +work and the 15 manifests that had it "wrong" needed no edit. The alternative — +reordering every manifest — would have left the next one free to regress. **5 — ~~Regenerate `docs/API.md` (§3).~~ Done.** Rewritten against the source rather than patched, and extended to the exports that never had an entry (variables, history, prompts, the authoring package). One new finding came out of -it: `CubePackageRef` is not re-exported from `src/index.ts` although `NopyConfig` -refers to it — a one-line fix, left for whoever next touches the export list. +it — `CubePackageRef` was not re-exported from `src/index.ts` although +`NopyConfig` refers to it — and that one line has since been added. -**6 — Cube docs (§5) and the two missing READMEs.** `service/autostart` is the -worst — its README belongs to a different cube, and its `deploy.py` does not run -at all (§6.1). +**6 — Cube docs (§5).** `service/autostart` was the worst and is **done**: its +`deploy.py` now reads its three variables off `host.data` instead of raising +`NameError` (§6.1), and its README describes that cube rather than a different +one (§5.1). Still open: §5.2 (four wrong parameters in +`network/wifi/access-point`), §5.3 (two cubes claiming to have no parameters) and +the missing READMEs. -**7 — Secrets on stdout (§4.2, §6.4).** The `console.log` in `Variables.assign` -is gone. Still open: mask the password in the executor's debug line and in the -dry-run plan, and pass `--user`/`--password` as argv rather than interpolating -into a shell string. +**7 — ~~Secrets on stdout (§4.2, §6.4).~~ Done as far as it can be.** The +`console.log` in `Variables.assign` is gone; the password is masked in the +executor's debug line and in the dry-run plan; and the whole command is argv now, +run without a shell, so nothing is interpolated into a string any shell will +re-parse. What is left is inherent: pyinfra takes `--data` on its own argv, so +the value is visible in `ps` on the machine running the deploy for the length of +the run. Fixing that means a change on pyinfra's side, not this one's. §6.4's +remaining bullet is unrelated debug output. diff --git a/packages/nopy/README.md b/packages/nopy/README.md index 93d2ae0..4b4c6cd 100644 --- a/packages/nopy/README.md +++ b/packages/nopy/README.md @@ -1,28 +1,30 @@ # Nopy -A CLI tool that simplifies **pyinfra** script management and execution, providing an interactive workflow for deploying infrastructure configurations ("cubes") to remote hosts. +A CLI tool that simplifies **pyinfra** script management and execution, providing an interactive workflow for deploying infrastructure configurations `cubes` to remote hosts. ## Overview -Nopy wraps pyinfra with structure, validation, and an interactive experience for managing complex infrastructure deployments. It organizes deployments into self-contained "cubes" with dependency management, schema validation, and lifecycle hooks. +Nopy wraps [pyinfra](https://pyinfra.com/) in the javascript ecosystem to provide an interactive experience for managing repeatable infrastructure deployments. It organizes deployments into self-contained units - called `cubes` - adding support for transitive dependency management, user input validation, and different lifecycle hooks. -## Features +## Features in a Nutshell -- **Dependency resolution** with topological sorting -- **Before/after hooks** for multi-cube orchestration +- **Manifest files** to support declarative description of user inputs and orchestration semantics per cube +- **Dependency resolution** in dependency order, with cycle detection +- **Before/after hooks** for programmable, multi-cube orchestration - **SSH key or password authentication** - **Default values** with optional customization via manifest `env` -- **Schema validation** using Zod +- **Schema validation** and **type coercion** using Zod - **Recursive cube directory discovery** -- **Dry-run mode** for previewing deployments +- **Dry-run mode** for previewing deployment scenarios - **Pipeable output** for CI/CD integration — the plan on stdout, everything else on stderr -- **Session history** with replay capability +- **Session history** for fast replay during development +- **Multi-layered** config files with natural discovery and deterministic parameter resolution ## Workflow 1. **Load cubes** - Discovers and validates cubes from configured directories 2. **Interactive prompts** - Select cubes, target host, and authentication method -3. **Dependency resolution** - Topologically sorts cubes based on dependencies +3. **Dependency resolution** - Resolves each cube's dependencies before the cube itself, so the deploy order is a topological order of the graph; a cycle is reported by name rather than recursed into 4. **Variable assignment** - Validates and collects configuration with schema validation 5. **Execute hooks** - Runs before/after hooks for orchestration 6. **Deploy** - Sequentially executes pyinfra commands @@ -33,23 +35,20 @@ Nopy wraps pyinfra with structure, validation, and an interactive experience for A cube is a **directory** containing two files: -- **JavaScript manifest**: `manifest.mjs` defining schema, dependencies, defaults, secrets, and hooks +- **JavaScript manifest**: `manifest.mjs` defining schema, dependencies, defaults, secrets (encrypted only), and hooks - **Python deployment script**: `deploy.py`, a plain pyinfra script Configuration variables are declared in the manifest and validated with Zod schemas before the deployment script runs. ``` cubes/ -├── .npcubes └── apt/ └── install/ ├── manifest.mjs └── deploy.py ``` -Any directory holding both files is treated as a cube, so cubes can be nested as deeply as you like to group them by topic. Discovery is recursive; directories starting with `.` and `node_modules` are skipped. Additional files in the cube directory (a `README.md`, config templates, and so on) are ignored by the loader and can be referenced from the deploy script — the script runs with its cube directory as the working directory. - -The prefixed forms `.manifest.mjs` and `.deploy.py` are also still recognized, but plain `manifest.mjs` / `deploy.py` is the current convention. +Any directory holding both files is treated as a cube, so cubes can be nested and grouped by topic. Discovery is recursive; hidden directories starting with `.` and `node_modules` are skipped. Additional files in the cube directory (a `README.md`, config templates, and so on) are ignored by the loader but can be referenced from the deploy script — ** the pyinfra script runs with its cube directory as the working directory**. A cube's identity comes from the manifest's `id` field (see below). If `id` is omitted, nopy falls back to an `[id]` prefix in the manifest `name`, and finally to the directory's own name. Note that the id does not have to mirror the folder path — `cubes/network/tailscale` declares `id: 'net:tailscale'`. @@ -65,7 +64,7 @@ export default cubes.Manifest({ dependencies: () => [], schema: z.object({ UPDATE: z.boolean().describe('Update package cache').default(false), - PACKAGES: z.string().describe('Space-separated list of packages').default('vim htop'), + PACKAGES: z.string().describe('Space-separated list of packages').default('curl htop'), }) }) ``` @@ -118,7 +117,7 @@ A variable can be set from several places in one run. Every assignment is kept, This allows cubes to ship with reasonable defaults while still allowing users to override them globally via `.nopyrc.json` or interactively during deployment. Because `env` outranks the schema, `.nopyrc.json` is also what steers a run started with `--use-defaults`, which never prompts. -`prompt` and `param` rarely compete: a key a dependency supplies is left out of the prompt entirely, so the user is only ever asked about the keys nothing else has set. +`prompt` and `param` rarely compete: a key supplied by a dependency is left out of the user input prompt entirely. Ranking by origin rather than by arrival order is what makes replay work: a recorded value is applied *before* the cube would be prompted for, and prompting can still override it, but a `--data` value pushed in by a dependency is never clobbered by a stale recording. @@ -572,7 +571,7 @@ A `--load-session` run *is* recorded, and the distinction is the point: a sessio A run is *not* recorded when: -- `--dry-run`, `--print-only` or `--no-history` is passed — the first two deploy nothing, and history is what `-R` repeats +- `--dry-run`, `--print-only` or `--no-save-history` is passed — the first two deploy nothing, and history is what `-R` repeats - No cubes were selected, so there was nothing to deploy - `history.autoSave` is set to `false` in `.nopyrc.json` - it is a `-R` or `-H` replay, as above diff --git a/packages/nopy/docs/API.md b/packages/nopy/docs/API.md index a6ff95f..63a5941 100644 --- a/packages/nopy/docs/API.md +++ b/packages/nopy/docs/API.md @@ -439,21 +439,30 @@ Recursive, per (cube, host): 5. emit the deploy call; 6. run `after` hooks. -There is no separate topological sort — the ordering falls out of the recursion, -and a `${cubeId}:${host}` set makes emission idempotent. Consequently there is no -cycle detection either: two mutually dependent cubes recurse until the stack -overflows. +There is no separate topological sort — emission is post-order, so a dependency +is always emitted ahead of its dependent and the ordering *is* topological +without an algorithm computing it. A `${cubeId}:${host}` set makes emission +idempotent. -**Throws** when the cube id is unknown, when `useDefaults` cannot fill a required -key, when a replay would need a value only the user has (secrets are never -recorded), and when a cancelled prompt leaves a required key empty. +Cycles are detected by the resolution stack rather than by the sort that does not +exist: a (cube, host) pair re-entered while it is still resolving raises with the +whole path named — `Circular dependency on host1: a → b → c → a`. The stack is +separate from the idempotence set on purpose, since re-entering a *finished* cube +with different `param` overrides is legitimate and a dependency or hook may do it. -The command it builds: +**Throws** when the cube id is unknown, when the dependency graph contains a +cycle, when `useDefaults` cannot fill a required key, when a replay would need a +value only the user has (secrets are never recorded), and when a cancelled prompt +leaves a required key empty. + +The command it builds — an argv array, one element per argument, nothing quoted: ``` -pyinfra -y [--user U --password P] --data "K=V" … --chdir / +pyinfra -y [-v|-vv|-vvv] [--debug] [--user U --password P] --data K=V … --chdir / ``` +The verbosity and debug flags come from `config.log` via `logConfigToFlags()`. + --- ## Variables Module @@ -599,9 +608,14 @@ interface ExecutionOptions { ### `executeDeployCalls(calls, options?)` Runs the calls **sequentially**, in the order they were built, through -`execa({ shell: true })` with `stdio: 'inherit'` so pyinfra's output reaches the -terminal live. Stops at the first failure unless `continueOnError`. With -`dryRun`, prints the plan and returns `[]` without executing. +`execa(command[0], command.slice(1))` with `stdio: 'inherit'` so pyinfra's output +reaches the terminal live. Stops at the first failure unless `continueOnError`. +With `dryRun`, prints the plan and returns `[]` without executing. + +**No shell.** It used to join `command` into one string and run it through +`execa({ shell: true })`, which made every `--data` value shell syntax: a +password or a variable containing `;`, a backtick or `$(…)` was executed rather +than passed along. Spawning the argv directly removes the parse step entirely. ```typescript const results = await executeDeployCalls(calls, { @@ -630,9 +644,11 @@ maskVariables(call); // Record pyinfra takes its data on the command line, so the real values have to be in `call.command`; these are the last point before they would reach a log, a -`--print-only` dump or a dry-run plan. `maskCommand` replaces the SSH -`--password` argument and every `--data "KEY=…"` whose key the manifest declared -a secret. +`--print-only` dump or a dry-run plan. `maskCommand` walks the argv, replaces the +element after `--password` and the value of every `--data KEY=…` whose key the +manifest declared a secret, and shell-quotes the rest so `--print-only` output +stays pasteable. It is the only thing that joins `command` into a string — +nothing executes it that way. This covers nopy's own output only. The value still reaches pyinfra on its command line, so it is visible in `ps` — inherent to pyinfra's `--data` @@ -808,7 +824,7 @@ interface SessionHistory { | `formatHistoryList(entries)` | `string` | what `nopy history` prints | Recording is suppressed for a dry run, a print-only run, a `-R`/`-H` replay out -of history, a run that built no deploy calls, `--no-history`, and +of history, a run that built no deploy calls, `--no-save-history`, and `history.autoSave: false` in the config. A `--load-session` run **is** recorded: it is not in history already, and without the entry `-R` would have nothing to repeat. @@ -870,9 +886,8 @@ config's `node_modules` rather than the working directory's. It is the same problem `PATH_PROPERTIES` solves for relative `cubeDirs`, with a different answer: a reference to resolve later instead of a rewritten path. -> The `CubePackageRef` name is currently not re-exported from the package root, -> though `NopyConfig` refers to it. Import it from `@bitsquare/nopy` and you get -> `NopyConfig` but not this type by name. +Re-exported from the package root alongside `NopyConfig`, which refers to it — +it was not, until the regeneration of this document noticed. ### `loadConfig()` @@ -1105,7 +1120,7 @@ nopy install -l ./sess.json # replay a session file nopy install -n # dry run — print the plan, execute nothing nopy install -P # print the built pyinfra commands and exit nopy install -c # continue after a failure -nopy install --no-history # do not record this run +nopy install --no-save-history # do not record this run nopy history # list recorded sessions (alias: h; -j for JSON) nopy clear-history # drop them all @@ -1125,8 +1140,10 @@ Exit code is 1 when any cube failed. "up to date", since an unanswerable check is not a negative answer. See [Known gaps](#known-gaps) for what that message conflates. -> `-H ` and `--no-history` share one Commander destination, so passing both -> discards the id and falls through to an interactive run. +> The suppression flag is `--no-save-history`, not `--no-history`. Commander +> derives an option's destination from its long flag with `no-` stripped, so +> `--no-history` wrote to the same `options.history` that `-H ` does and +> `nopy install -H abc --no-history` silently discarded the id. --- @@ -1169,12 +1186,15 @@ export default Manifest({ }); ``` -> **Call `.default()` before `.describe()`.** In zod 4, `.default()` returns a -> `ZodDefault` wrapper that does not inherit `.description` from the type it -> wraps, and the prompt reads the description off the outer node. So -> `z.boolean().describe('Update cache').default(false)` prompts with the bare key -> `UPDATE`, while `z.boolean().default(false).describe('Update cache')` prompts -> with the sentence. Verified against zod 4.4.3. +> **The order of `.default()` and `.describe()` does not matter.** It used to. +> In zod 4, `.default()` returns a `ZodDefault` wrapper that does not inherit +> `.description` from the type it wraps, so +> `z.boolean().describe('Update cache').default(false)` prompted with the bare +> key `UPDATE` while the other order prompted with the sentence — a difference +> nothing announced, and one that 15 of the 22 core cubes were on the wrong side +> of. The prompt now unwraps `default`/`optional`/`nullable` looking for a +> description, so either chaining order gives the label. Verified against +> zod 4.4.3. Every schema key reaches pyinfra as `--data KEY=value`, so `host.data.KEY` is always defined. pyinfra parses the values itself: `"true"` arrives as a bool and @@ -1206,11 +1226,6 @@ For packaging cubes as an installable npm bundle, see Real behaviour that a reader would otherwise take on trust. Tracked in `DOCS-AUDIT.md` and summarised in `CLAUDE.md`. -- **`logConfigToFlags()` is never consumed.** It is exported and unit-tested, but - nothing feeds its output into the built pyinfra command, so `log.verbosity` and - `log.debug` in `.nopyrc.json` have no effect today. -- **No cycle detection.** Ordering is a side effect of recursion, not a - topological sort. Two mutually dependent cubes overflow the stack. - **`DeployCall.dependencies` is always `[]`.** The field is populated nowhere; dependency information lives in the emission order. - **`ExecutionResult.stdout` / `.stderr` are always `undefined`,** because the diff --git a/packages/nopy/src/index.ts b/packages/nopy/src/index.ts index 16ade49..6aec811 100644 --- a/packages/nopy/src/index.ts +++ b/packages/nopy/src/index.ts @@ -10,6 +10,7 @@ export type { Assignment, Origin, TVariables, Value } from './nopy.common.js'; // Variables export { MASK, Variable, Variables } from './nopy.common.js'; export type { + CubePackageRef, ExecutionConfig, HistoryConfig, LogConfig,