diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..d40e204 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,28 @@ +# Keeps GitHub Actions current so a runtime deprecation is a pull request, not a +# warning nobody sees. +# +# The Node 20 deprecation is the case this exists for: five actions kept working +# while the runners forced them onto Node 24, and the only signal was a notice in +# the log of a workflow that still passed. Pinned majors do not rot loudly — they +# rot silently until the forced runtime is withdrawn and every workflow fails at +# once. A weekly PR turns that into something reviewable. +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + open-pull-requests-limit: 5 + commit-message: + # Matches the Conventional Commits style the rest of the history uses, so + # these land in the CHANGELOG tooling like anything else. + prefix: ci + labels: + - dependencies + groups: + # One PR for all of them: they are bumped together in practice (upload and + # download artifact must stay compatible), and five separate PRs for the + # same deprecation is five reviews of the same decision. + actions: + patterns: + - "*" diff --git a/.github/workflows/php-analysis.yml b/.github/workflows/php-analysis.yml index fed0461..28b1bf3 100644 --- a/.github/workflows/php-analysis.yml +++ b/.github/workflows/php-analysis.yml @@ -79,7 +79,7 @@ jobs: SEMGREP_RULES: "p/php p/security-audit p/secrets" - name: Upload SARIF to code scanning if: always() - uses: github/codeql-action/upload-sarif@v3 + uses: github/codeql-action/upload-sarif@v4 with: sarif_file: semgrep.sarif category: semgrep diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a6502d5..b5ad55b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -66,12 +66,12 @@ jobs: # TWO artifacts. The tarball is the DEFAULT install path — user-local, # no root (tools/install.sh). The .deb is for multi-user machines and CI # images where a system-wide install and apt-managed PHP are the point. - - uses: actions/upload-artifact@v5 + - uses: actions/upload-artifact@v7 with: { name: linux-tarball, path: dist/*linux*.tar.gz } - - uses: actions/upload-artifact@v5 + - uses: actions/upload-artifact@v7 with: { name: linux-deb, path: dist/*.deb } # Published alongside the assets so `curl | sh` works without a checkout. - - uses: actions/upload-artifact@v5 + - uses: actions/upload-artifact@v7 with: { name: installer, path: tools/install.sh } # ── Windows: .zip (x86_64, cross-compiled) ──────────────────────────────── @@ -89,7 +89,7 @@ jobs: with: { php-version: "8.4", tools: composer } - name: Bundle (windows) run: VERSION="${RELEASE_VERSION:-${GITHUB_REF_NAME#v}}" ./tools/bundle.sh windows - - uses: actions/upload-artifact@v5 + - uses: actions/upload-artifact@v7 with: { name: windows-zip, path: dist/*.zip } # ── macOS: universal .app tarball (arm64 + x86_64), built ON LINUX ───────── @@ -109,7 +109,7 @@ jobs: with: { php-version: "8.4", tools: composer } - name: Bundle (macos) run: VERSION="${RELEASE_VERSION:-${GITHUB_REF_NAME#v}}" ./tools/bundle.sh macos - - uses: actions/upload-artifact@v5 + - uses: actions/upload-artifact@v7 with: { name: macos-app, path: dist/*.tar.gz } # ── Publish GitHub Release with all artifacts ───────────────────────────── @@ -119,7 +119,7 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v5 # need CHANGELOG.md at the checked-out commit - - uses: actions/download-artifact@v5 + - uses: actions/download-artifact@v8 with: { path: artifacts/ } - name: Resolve version + extract CHANGELOG section id: notes @@ -137,7 +137,7 @@ jobs: else echo "No CHANGELOG entry for $VERSION — using auto-generated notes only." fi - - uses: softprops/action-gh-release@v2 + - uses: softprops/action-gh-release@v3 with: # When called from auto-release the ref is a branch, so name the tag # explicitly; on a tag push this matches GITHUB_REF_NAME anyway. diff --git a/CHANGELOG.md b/CHANGELOG.md index 778e89f..a96cec7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,105 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.3.3] - 2026-08-18 + +Follows 1.3.2 within a day, and the theme is narrower: 1.3.2 fixed *which* +kernel a command acts on, this one fixes *how* commands talk to the shell around +them, plus a full uninstall. An audit of the Zig toolchain drove it — every item +below was reproduced against the shipped `--release=small` binary. + +### Added +- **`hkm uninstall`** — removes every hkm install on the machine (both kernels, + both pairs of launchers, the pre-1.4 user kernel, `~/.config/hkm` and the + shared plugin store) and deregisters the `.deb` from dpkg, while keeping **your + projects and the project registry**. Those two are protected by construction, + not by a filter: every path it can delete is computed from the install layout, + so a project directory cannot enter the plan at all; and `projects.json` + + `platform.json` are rescued out of a kernel tree into the userdata directory + *before* anything is deleted, so the registry survives even when its only copy + was inside the tree being removed. `--dry-run` prints the plan and exits; + system paths are reported rather than silently skipped when not root. + +### Changed +- `hkm doctor` and `hkm version` share one PATH lookup with the launcher + passthrough (`util.findOnPath`). Three private copies of "which binary would + actually run" is three chances to disagree. +- **CI actions moved off the deprecated Node 20 runtime.** `upload-artifact` + v5→v7, `download-artifact` v5→v8, `codeql-action/upload-sarif` v3→v4 and + `action-gh-release` v2→v3 all declared `node20`, which the runners were + already forcing onto Node 24. A `.github/dependabot.yml` now watches the + `github-actions` ecosystem weekly and groups the bumps into one PR, so the + next runtime deprecation arrives as a reviewable change rather than a notice + in the log of a workflow that still passes. + +### Fixed +- **Command output went to stderr, so nothing could be piped.** The whole + `prompt` renderer used `std.debug.print`, which writes to stderr — so + `hkm list > projects.txt` produced an empty file and a command's results were + indistinguishable from its errors. Results (`intro`/`section`/`item`/`ok`/ + `muted`/`note`/`table`/`outro`) now go to **stdout**; `err`/`warn` and every + interactive prompt stay on **stderr**. This was found once before and fixed a + single function wide (`banner.printShort`); the cause was in the shared + renderer all along. +- **ANSI escapes were emitted unconditionally and `NO_COLOR` was ignored**, so + colour codes landed in redirected output, log files and CI transcripts. + Colour is now decided per stream from the rule `tools/install.sh` already + applied: off when `NO_COLOR` is set, when `TERM=dumb`, or when that stream is + not a terminal. +- **Tables truncated to 80 columns when redirected.** `termCols()` falls back to + 80 whenever the `ioctl` fails — exactly the non-TTY case — so piping cut the + end off every long path, with the `…` as the only clue. Truncation now applies + only when stdout really is a terminal. +- **A mistyped command printed a raw Zig error.** Anything not handled natively + is forwarded to the PHP CLI, and a spawn failure propagated out of `main` as + `error: FileNotFound` — no filename, no mention of PHP, no pointer to + `hkm doctor`. The three causes (no PHP, no kernel CLI, an unknown command) are + now told apart and each names its own fix. +- **Unknown flags were silently ignored, which inverted a destructive command.** + Every command parsed the flags it knew and dropped the rest. The token most + likely to be misspelled is the one that makes a command safe, so + `hkm uninstall --dryrun --yes` parsed as "no dry run, and don't ask" and + deleted the install without a prompt. `uninstall` and `upgrade` now reject + anything they do not recognise before acting on anything they do. +- **`projects.json` and `plugins.lock.json` were written non-atomically.** + `writeFile` truncates before writing, so a process killed part way through — or + a full disk — left a truncated registry rather than the previous one. Both now + write a sibling temp file and `rename()` over the target, the same pattern + `install.sh` and the launcher install already used. +- **Every unknown long option crashed the CLI parser.** `php-io-cli`'s + long-option branch recorded a different array shape than its two siblings, and + `rejectUnknownOptions()` reads the key it omitted — so the feature meant to + suggest a correction raised `TypeError: suggestOption(): Argument #1 ($name) + must be of type string, null given` on every unknown `--flag`. Fixed upstream + (php-io-cli `b1dd657`) rather than pinned back, so the handling stays in. +- **`hkm uninstall` could destroy the registry it promises to keep** (found in + review). Two holes: `HKM_USERDATA_DIR` may point INSIDE a deletion target — + `/opt/hkm-kernel/projects` is the obvious case — so the plan listed it under + "Will KEEP" and deleted its parent moments later; and `rescueRegistry` + swallowed every write failure, so a failed rescue was followed by the delete + anyway while the command reported success. It now refuses the first layout + outright and aborts before removing anything if the rescue fails. Rescued + files are written atomically. +- **Output fixes that had gaps of their own** (found in review): `hkm-config + print` still wrote the config to stderr; a line longer than 8 KiB fell back to + `std.debug.print` and silently changed stream; remediation text printed after + an error went to stdout, splitting one message across two streams; + `writeFileAtomic` used a fixed temp name two processes could collide on; + `findOnPath` skipped empty `PATH` entries, which POSIX defines as the current + directory; the passthrough blamed a missing PHP for a missing kernel CLI; and + `--` ended flag VALIDATION but not flag PARSING, so `hkm upgrade -- --system` + still selected the system scope. +- **A failed `.deb` install could still report success.** The fallback path + treated `apt-get -f install` exiting 0 as evidence the package had landed, but + it exits 0 whenever it finds nothing to repair — so a `dpkg -i` that failed for + any non-dependency reason (a truncated download, a corrupt `.deb`) was reported + as "updated" with the previous kernel still installed. The dependency repair is + now followed by a second `dpkg -i`, and that result alone is the verdict. +- **`prompt.item` padded by byte count**, so a key containing any multi-byte + glyph shifted its description column left — a single `→` misaligned the row by + two. It now measures display width using the helper already written for + `table()`. + ## [1.3.2] - 2026-08-17 Fixes a class of failure that made installing or upgrading on a machine with an diff --git a/modules/php-io-cli b/modules/php-io-cli index 53620ec..b1dd657 160000 --- a/modules/php-io-cli +++ b/modules/php-io-cli @@ -1 +1 @@ -Subproject commit 53620ec587ac8cf29ce82f239e072f566d73c79c +Subproject commit b1dd65744984bd3ac46e31069d21e93aef724054 diff --git a/tools/README.md b/tools/README.md index 6a777e0..57d2a54 100644 --- a/tools/README.md +++ b/tools/README.md @@ -146,6 +146,40 @@ So a pin still works wherever it was actually needed; it no longer overrides an install sitting next to the binary. `hkm-config check` writes one only when self-location failed, and `hkm-config unset HKM_KERNEL_HOME` clears a stale one. +### `hkm uninstall` — remove everything, keep the projects + +```sh +hkm uninstall --dry-run # print the plan, delete nothing +hkm uninstall # everything this user can remove +sudo hkm uninstall # …including the .deb under /opt + /usr/bin +``` + +| Removed | Kept | +|---|---| +| `/opt/hkm-kernel`, `~/.local/lib/hkm-kernel`, the pre-1.4 user kernel | **your projects**, wherever they live | +| `/usr/bin/{hkm,hkm-config}`, `~/.local/bin/{hkm,hkm-config}` | **`projects.json`** + **`platform.json`** | +| `~/.config/hkm` (config.env), the shared plugin store | | +| the `hkm-kernel` dpkg registration (`remove`, never `purge`) | | + +The two kept items are protected **by construction, not by a filter**: + +- Every path the command can delete is *computed* from the install layout. None + is read from the registry, the working directory, or an argument — so a + project directory cannot appear in the plan at all. +- `projects.json` and `platform.json` are **rescued into the userdata directory + before anything is deleted**, so the registry survives even when its only copy + was inside the kernel tree being removed. Reinstall later and `hkm list` still + shows every project. + +Two rules in the rescue exist because testing found them the hard way: it only +copies from a tree it is *actually removing*, and it prefers the user's kernel +over the system one. The `.deb` ships `projects/projects.json` as `{}` (a +packaged conffile), and without both rules that empty default won the race to the +destination and shadowed the user's real project list. + +`tools/install.sh --uninstall` remains deliberately narrow — it removes only what +that script installed, at that prefix. `hkm uninstall` is the full removal. + ### `hkm upgrade --local` Installs the current checkout over an installed kernel, obeying the same scope diff --git a/tools/install.sh b/tools/install.sh index 23617d6..b963fb1 100755 --- a/tools/install.sh +++ b/tools/install.sh @@ -73,13 +73,20 @@ done # ── uninstall ─────────────────────────────────────────────────────────────── if [ "$DO_UNINSTALL" -eq 1 ]; then + # Deliberately NARROW: this removes only what this script installed, at this + # prefix. It does not touch a system (.deb) install, the pre-1.4 user kernel, + # the config file or the plugin cache — `hkm uninstall` does all of that, and + # rescues the project registry out of any kernel tree before deleting it. say "Removing $DEST" rm -rf "$DEST" rm -f "$BINDIR/hkm" "$BINDIR/hkm-config" ok "Removed. Your data was left alone:" printf ' %s\n %s\n' "${XDG_CONFIG_HOME:-$HOME/.config}/hkm" \ "${XDG_DATA_HOME:-$HOME/.local/share}/hkm" - printf ' Delete those too if you want a clean slate.\n' + printf '\n For a FULL removal — every install on this machine, the config and\n' + printf ' the plugin cache, while keeping your projects and projects.json:\n' + printf ' hkm uninstall --dry-run # see the plan first\n' + printf ' hkm uninstall # then do it (sudo for the .deb too)\n' exit 0 fi diff --git a/tools/src/commands/doctor.zig b/tools/src/commands/doctor.zig index 2332ca2..a897f90 100644 --- a/tools/src/commands/doctor.zig +++ b/tools/src/commands/doctor.zig @@ -56,31 +56,11 @@ fn mark(present: bool) []const u8 { return if (present) OK else MISSING; } -/// Locate an executable by walking PATH. Returns the FIRST match, which is the -/// one that would actually run. -fn findOnPath(allocator: std.mem.Allocator, io: Io, env: *EnvMap, name: []const u8) ?[]const u8 { - const path = env.get("PATH") orelse return null; - var it = std.mem.splitScalar(u8, path, ':'); - while (it.next()) |dir| { - if (dir.len == 0) continue; - const cand = std.fs.path.join(allocator, &.{ dir, name }) catch continue; - if (util.fileExists(io, cand)) return cand; - } - return null; -} - -/// Every match on PATH, in order — used to detect one install shadowing another. -fn countOnPath(allocator: std.mem.Allocator, io: Io, env: *EnvMap, name: []const u8) usize { - const path = env.get("PATH") orelse return 0; - var n: usize = 0; - var it = std.mem.splitScalar(u8, path, ':'); - while (it.next()) |dir| { - if (dir.len == 0) continue; - const cand = std.fs.path.join(allocator, &.{ dir, name }) catch continue; - if (util.fileExists(io, cand)) n += 1; - } - return n; -} +// findOnPath / countOnPath now live in lib/util.zig — doctor, version and the +// passthrough diagnostic in main.zig all needed the same walk, and three copies +// of "which binary would actually run" is three chances to disagree. +const findOnPath = util.findOnPath; +const countOnPath = util.countOnPath; fn dirOnPath(env: *EnvMap, dir: []const u8) bool { const path = env.get("PATH") orelse return false; diff --git a/tools/src/commands/uninstall.zig b/tools/src/commands/uninstall.zig new file mode 100644 index 0000000..1ed9491 --- /dev/null +++ b/tools/src/commands/uninstall.zig @@ -0,0 +1,634 @@ +//! `hkm uninstall` — remove every trace of hkm from this machine, EXCEPT the +//! project registry and the projects themselves. +//! +//! hkm uninstall # remove everything present that this user can +//! sudo hkm uninstall # …including the system (.deb) install +//! hkm uninstall --dry-run # show the plan, delete nothing +//! +//! WHAT IS NEVER TOUCHED +//! --------------------- +//! Two things, and they are protected by CONSTRUCTION rather than by a filter: +//! +//! 1. **Your projects.** hkm never owned them. Every path this command can +//! delete is COMPUTED from the install layout (lib/install_scope.zig, +//! lib/userconfig.zig, lib/plugin_store.zig) — none of them is read from +//! the registry, from the current directory, or from an argument. A +//! project directory therefore cannot appear in the plan at all. +//! 2. **projects.json + platform.json.** Before any kernel root is removed, +//! its copy of the registry is RESCUED into the userdata directory, and +//! that directory is never itself a target — only the stale kernel that +//! may sit inside it. So the registry survives even when the only copy was +//! inside the tree being deleted. +//! +//! A machine can hold two installs (system `.deb` and user tarball — see +//! lib/install_scope.zig), plus config, plus a plugin cache. "Uninstall" that +//! removes only one of those leaves the others to be discovered months later, +//! which is why the default here is everything this user is able to remove +//! rather than a single scope. +//! +//! The command deletes the binary it is running from. On Linux that is safe: +//! unlinking an executable does not disturb the running process, which finishes +//! normally. + +const std = @import("std"); +const install_scope = @import("../lib/install_scope.zig"); +const prompt = @import("../lib/prompt.zig"); +const run_cmd = @import("run.zig"); +const userconfig = @import("../lib/userconfig.zig"); +const util = @import("../lib/util.zig"); + +const Dir = std.Io.Dir; +const Io = std.Io; +const EnvMap = std.process.Environ.Map; + +/// One thing to delete. +const Target = struct { + path: []const u8, + /// Human label for the plan table. + what: []const u8, + is_dir: bool, + present: bool, + /// Removing it needs root (a system path). + needs_root: bool, +}; + +/// The registry filenames that must outlive the uninstall. +const registry_files = [_][]const u8{ "projects.json", "platform.json" }; + +pub fn run(allocator: std.mem.Allocator, io: Io, env: *EnvMap, args: []const []const u8) !u8 { + // Reject anything not recognised BEFORE acting on what is. + // + // The house style elsewhere is to ignore unknown flags so future ones do not + // hard fail. That is wrong for this command: the token most likely to be + // misspelled is the one that makes it safe, and ignoring it inverts the + // meaning of the line. `hkm uninstall --dryrun --yes` parsed as "no dry run, + // and don't ask" and deleted the install without a prompt. + const known = [_][]const u8{ "--dry-run", "-n", "--yes", "-y", "--help", "-h" }; + if (util.unknownFlag(args[1..], &known)) |bad| { + prompt.err(std.fmt.allocPrint(allocator, "unknown option: {s}", .{bad}) catch "unknown option"); + prompt.hintLine(" nothing was removed. Run `hkm uninstall --help` for the accepted flags."); + return 1; + } + + var dry_run = false; + var assume_yes = false; + for (args[1..]) |a| { + if (std.mem.eql(u8, a, "--")) break; // end of options, as unknownFlag treats it + if (std.mem.eql(u8, a, "--dry-run") or std.mem.eql(u8, a, "-n")) dry_run = true; + if (std.mem.eql(u8, a, "--yes") or std.mem.eql(u8, a, "-y")) assume_yes = true; + if (std.mem.eql(u8, a, "--help") or std.mem.eql(u8, a, "-h")) { + printHelp(); + return 0; + } + } + + const is_root = install_scope.isRoot(env); + prompt.intro("hkm uninstall"); + + // ── The directory that must survive ───────────────────────────────────── + const keep_dir = userdataDir(allocator, env); + + // ── Build the plan ────────────────────────────────────────────────────── + var targets: std.ArrayList(Target) = .empty; + + // System scope: the .deb's kernel and launchers. + try addTarget(allocator, io, &targets, install_scope.system_root, "system kernel", true, true); + for ([_][]const u8{ "hkm", "hkm-config" }) |name| { + const p = try std.fs.path.join(allocator, &.{ install_scope.system_bin_dir, name }); + try addTarget(allocator, io, &targets, p, "system launcher", false, true); + } + + // User scope: the tarball install, its launchers, and the pre-1.4 kernel. + if (install_scope.userRoot(allocator, env)) |root| { + try addTarget(allocator, io, &targets, root, "user kernel", true, false); + } + if (install_scope.userBinDir(allocator, env)) |bin| { + for ([_][]const u8{ "hkm", "hkm-config" }) |name| { + const p = try std.fs.path.join(allocator, &.{ bin, name }); + try addTarget(allocator, io, &targets, p, "user launcher", false, false); + } + } + if (install_scope.legacyUserRoot(allocator, env)) |legacy| { + try addTarget(allocator, io, &targets, legacy, "legacy user kernel", true, false); + } + + // Config: ~/.config/hkm (holds config.env). + if (configDir(allocator, env)) |cfg| { + try addTarget(allocator, io, &targets, cfg, "launcher config", true, false); + } + + // Plugin cache: the shared plugin store. + if (pluginStore(allocator, env)) |store| { + try addTarget(allocator, io, &targets, store, "plugin cache", true, false); + } + + var present: usize = 0; + var blocked: usize = 0; + for (targets.items) |t| { + if (!t.present) continue; + present += 1; + if (t.needs_root and !is_root) blocked += 1; + } + + if (present == 0) { + prompt.ok("nothing to uninstall — no hkm install found on this machine."); + if (keep_dir) |k| prompt.item("registry (untouched)", k); + return 0; + } + + // ── The registry must not live inside something we are about to delete ─── + // + // "Kept" and "removed" are only different outcomes while the kept path sits + // OUTSIDE every target. HKM_USERDATA_DIR is operator-settable and can point + // straight into one — `/opt/hkm-kernel/projects` is the obvious case, since + // that is where the registry lived before it was migrated out. The plan + // would then list that directory under "Will KEEP" and delete its parent a + // moment later, which is precisely the loss this command promises cannot + // happen. Refuse instead of guessing: moving the registry is the operator's + // decision, and either choice they make is destructive to get wrong. + if (keep_dir) |k| { + for (targets.items) |t| { + if (!t.present or !t.is_dir) continue; + if (t.needs_root and !is_root) continue; // not ours to delete + if (!util.isInside(k, t.path)) continue; + + prompt.err(try std.fmt.allocPrint( + allocator, + "the registry directory is inside a directory this would delete.", + .{}, + )); + prompt.item("registry", k); + prompt.item("would delete", t.path); + prompt.muted(" nothing was removed. Move the registry somewhere outside the install first:"); + prompt.muted(" hkm-config set HKM_USERDATA_DIR ~/.local/share/hkm # then re-run"); + return 1; + } + } + + // ── Show it ───────────────────────────────────────────────────────────── + prompt.section("Will remove"); + var rows: std.ArrayList([]const []const u8) = .empty; + for (targets.items) |t| { + if (!t.present) continue; + try rows.append(allocator, try allocator.dupe([]const u8, &.{ + t.what, + t.path, + if (t.needs_root and !is_root) "NEEDS ROOT — skipped" else "will be removed", + })); + } + prompt.table(allocator, &.{ "what", "path", "" }, rows.items); + + prompt.section("Will KEEP"); + if (keep_dir) |k| { + prompt.item("registry dir", k); + for (registry_files) |f| { + const p = try std.fs.path.join(allocator, &.{ k, f }); + prompt.item(f, if (util.fileExists(io, p)) "kept" else "not present"); + } + } else { + prompt.warn("could not resolve a registry directory (no HOME) — nothing to preserve."); + } + prompt.hintLine(" your projects are never touched: no path above is read from the registry,"); + prompt.hintLine(" the working directory, or an argument — every one is a fixed install path."); + + if (dry_run) { + prompt.blank(); + prompt.outro("Dry run — nothing was removed"); + return 0; + } + + prompt.blank(); + if (!assume_yes and !prompt.confirm(io, "Remove everything listed above?", false)) { + prompt.muted("cancelled — nothing was removed"); + return 1; + } + + // ── Rescue the registry BEFORE anything is deleted ─────────────────────── + // + // The only copy of projects.json may live inside a kernel tree that is about + // to go (that is where it lived before `hkm-config check` migrated it out). + // Copying first means the guarantee holds even then. + if (keep_dir) |k| { + rescueRegistry(allocator, io, k, targets.items, is_root) catch |e| { + prompt.err(try std.fmt.allocPrint( + allocator, + "could not preserve the project registry ({t}) — nothing was removed.", + .{e}, + )); + prompt.muted(" the registry is the one thing this command guarantees, so it stops"); + prompt.muted(" rather than delete a kernel whose copy it failed to save."); + return 1; + }; + } + + // ── Remove ─────────────────────────────────────────────────────────────── + prompt.section("Removing"); + + // Let dpkg forget the package first, so its database does not keep claiming + // an install that no longer exists. `remove`, never `purge`: purge deletes + // the conffiles, and projects.json + platform.json are marked as conffiles + // by tools/bundle.sh precisely because they are user data. + if (is_root) removeDebPackage(allocator, io, env); + + var removed: usize = 0; + var failed: usize = 0; + var skipped: usize = 0; + + for (targets.items) |t| { + if (!t.present) continue; + if (t.needs_root and !is_root) { + skipped += 1; + continue; + } + // Belt and braces. Every path here is computed, but the cost of a bug + // in that computation is an unrecoverable deletion, so the guard runs + // anyway and a rejected path is reported rather than silently skipped. + if (!removable(env, t.path)) { + failed += 1; + prompt.err(try std.fmt.allocPrint(allocator, "refused to remove an implausible path: {s}", .{t.path})); + continue; + } + + const result = if (t.is_dir) + Dir.cwd().deleteTree(io, t.path) + else + Dir.cwd().deleteFile(io, t.path); + + if (result) |_| { + removed += 1; + prompt.item("removed", t.path); + } else |e| { + // Already gone between the plan and now is success, not failure. + if (e == error.FileNotFound) { + removed += 1; + continue; + } + failed += 1; + prompt.err(try std.fmt.allocPrint(allocator, "{s}: {t}", .{ t.path, e })); + } + } + + // ── Report ─────────────────────────────────────────────────────────────── + prompt.blank(); + prompt.ok(try std.fmt.allocPrint(allocator, "removed {d} item(s)", .{removed})); + if (keep_dir) |k| { + prompt.item("registry kept at", k); + prompt.muted(" reinstall later and your projects are still registered: hkm list"); + } + + if (skipped > 0) { + prompt.blank(); + prompt.warn(try std.fmt.allocPrint( + allocator, + "{d} system item(s) were NOT removed — they need root.", + .{skipped}, + )); + prompt.item("finish with", "sudo hkm uninstall"); + prompt.muted(" if `hkm` is already gone from your PATH, use the full path:"); + prompt.muted(" sudo /usr/bin/hkm uninstall"); + return 1; + } + + if (failed > 0) { + prompt.err(try std.fmt.allocPrint(allocator, "{d} item(s) could not be removed.", .{failed})); + return 1; + } + + prompt.outro("hkm is uninstalled. Your projects and their registry were left alone."); + return 0; +} + +/// Record a target, resolving whether it is actually there. +fn addTarget( + allocator: std.mem.Allocator, + io: Io, + list: *std.ArrayList(Target), + path: []const u8, + what: []const u8, + is_dir: bool, + needs_root: bool, +) !void { + const p = util.trimSlash(path); + if (p.len == 0) return; + const there = if (is_dir) util.dirExists(Dir.cwd(), io, p) else util.fileExists(io, p); + try list.append(allocator, .{ + .path = try allocator.dupe(u8, p), + .what = what, + .is_dir = is_dir, + .present = there, + .needs_root = needs_root, + }); +} + +/// Copy any registry file found inside a doomed kernel tree into `keep`. +/// +/// Only fills gaps: a registry already in `keep` is the live one and is never +/// overwritten by a copy from inside an install being deleted. +/// +/// TWO RULES, both learned by running this against a machine that had both +/// installs. Without either, the rescue could REPLACE the user's real registry +/// with an empty default and report success: +/// +/// 1. **Only rescue from a tree that is actually being removed.** A system +/// kernel skipped for lack of root keeps its own registry where it is; +/// copying from it is not a rescue, it is an unrelated file winning the +/// race to an empty destination. +/// 2. **Prefer the user's kernel over the system one.** A `.deb` ships +/// `projects/projects.json` as `{}` (it is a packaged default, marked a +/// conffile). Scanning in target order put that `{}` first, and because the +/// destination then existed, the user's real project list was skipped. +fn rescueRegistry( + allocator: std.mem.Allocator, + io: Io, + keep: []const u8, + targets: []const Target, + is_root: bool, +) !void { + Dir.cwd().createDirPath(io, keep) catch |e| switch (e) { + error.PathAlreadyExists => {}, + // Nowhere to rescue TO. Continuing would delete the only copy. + else => return e, + }; + + // Pass 1 = the user's own kernels, pass 2 = system. Ordering is the fix for + // rule 2 above and must not be flattened back into one loop. + for ([_]bool{ false, true }) |system_pass| { + for (targets) |t| { + if (!t.present or !t.is_dir) continue; + if (t.needs_root != system_pass) continue; + // Only kernel roots carry a projects/ directory. + if (std.mem.indexOf(u8, t.what, "kernel") == null) continue; + // Rule 1: a tree we cannot remove keeps its own copy. + if (t.needs_root and !is_root) continue; + + for (registry_files) |f| { + const dest = try std.fs.path.join(allocator, &.{ keep, f }); + if (util.fileExists(io, dest)) continue; // the live copy wins + + const src = try std.fs.path.join(allocator, &.{ t.path, "projects", f }); + + // An ABSENT source is the only tolerable failure: this kernel + // simply has no registry, and the next one may. Every other + // error propagates, because the caller is about to delete this + // tree and a swallowed write error means the only copy is gone + // while the command reports success. + const data = Dir.cwd().readFileAlloc(io, src, allocator, .limited(8 * 1024 * 1024)) catch |e| switch (e) { + error.FileNotFound => continue, + else => return e, + }; + + // Atomic, for the same reason the registry writer is: a partial + // rescue is indistinguishable from a complete one afterwards. + try util.writeFileAtomic(io, dest, data); + prompt.item("rescued", dest); + } + } + } +} + +/// `apt-get remove` (falling back to `dpkg -r`) so dpkg stops believing the +/// package is installed. Best effort: a machine that installed from the tarball +/// has no package here, and that is not an error. +fn removeDebPackage(allocator: std.mem.Allocator, io: Io, env: *EnvMap) void { + if (@import("builtin").os.tag != .linux) return; + + // Only act when dpkg actually knows the package, so a tarball-only machine + // does not run apt for nothing (and does not print apt's error as if the + // uninstall had gone wrong). + const q = std.process.run(allocator, io, .{ + .argv = &.{ "dpkg-query", "-W", "-f=${Status}", "hkm-kernel" }, + .environ_map = env, + }) catch return; + switch (q.term) { + .exited => |c| if (c != 0) return, + else => return, + } + if (std.mem.indexOf(u8, q.stdout, "installed") == null) return; + + prompt.item("dpkg", "removing the hkm-kernel package"); + var apt = [_][]const u8{ "apt-get", "remove", "-y", "hkm-kernel" }; + if ((run_cmd.spawnWait(io, env, &apt) catch 1) == 0) return; + + var dpkg = [_][]const u8{ "dpkg", "-r", "hkm-kernel" }; + _ = run_cmd.spawnWait(io, env, &dpkg) catch {}; +} + +/// `$HKM_USERDATA_DIR`, else `$XDG_DATA_HOME/hkm`, else `~/.local/share/hkm`. +/// +/// Mirrors config.zig's ensureUserdata so the directory preserved here is +/// exactly the one the registry was migrated into. +fn userdataDir(allocator: std.mem.Allocator, env: *EnvMap) ?[]const u8 { + if (env.get("HKM_USERDATA_DIR")) |d| { + if (d.len > 0) return util.trimSlash(d); + } + if (env.get("XDG_DATA_HOME")) |x| { + if (x.len > 0) return std.fmt.allocPrint(allocator, "{s}/hkm", .{util.trimSlash(x)}) catch null; + } + const home = install_scope.homeDir(allocator, env) orelse return null; + return std.fmt.allocPrint(allocator, "{s}/.local/share/hkm", .{home}) catch null; +} + +/// `$XDG_CONFIG_HOME/hkm`, else `~/.config/hkm` — the directory holding +/// config.env. Mirrors lib/userconfig.zig's path(). +fn configDir(allocator: std.mem.Allocator, env: *EnvMap) ?[]const u8 { + if (env.get("XDG_CONFIG_HOME")) |x| { + if (x.len > 0) return std.fmt.allocPrint(allocator, "{s}/hkm", .{util.trimSlash(x)}) catch null; + } + const home = install_scope.homeDir(allocator, env) orelse return null; + return std.fmt.allocPrint(allocator, "{s}/.config/hkm", .{home}) catch null; +} + +/// The shared plugin store. Mirrors lib/plugin_store.zig's root(). +fn pluginStore(allocator: std.mem.Allocator, env: *EnvMap) ?[]const u8 { + if (env.get("HKM_PLUGIN_STORE")) |v| { + if (v.len > 0) return util.trimSlash(v); + } + if (env.get("XDG_CACHE_HOME")) |x| { + if (x.len > 0) return std.fmt.allocPrint(allocator, "{s}/hkm/plugin-store", .{util.trimSlash(x)}) catch null; + } + const home = install_scope.homeDir(allocator, env) orelse return null; + return std.fmt.allocPrint(allocator, "{s}/.cache/hkm/plugin-store", .{home}) catch null; +} + +/// Could this plausibly be an hkm-owned path? +/// +/// Pure defence in depth — every path reaching it was computed from a fixed +/// layout, so a rejection means a bug upstream. It exists because the cost of +/// that bug is an unrecoverable `deleteTree`, and three cheap invariants rule +/// out every catastrophic target: a relative path, a top-level directory, the +/// user's home itself, and anything with no `hkm` in it at all. +fn removable(env: *EnvMap, path: []const u8) bool { + if (path.len < 2 or path[0] != '/') return false; + + // At least two components, so "/" and "/opt" can never be targets. + var comps: usize = 0; + var it = std.mem.tokenizeScalar(u8, path, '/'); + while (it.next()) |_| comps += 1; + if (comps < 2) return false; + + // Never the home directory itself, however it was derived. + if (env.get("HOME")) |h| { + if (h.len > 0 and std.mem.eql(u8, util.trimSlash(h), path)) return false; + } + + return std.mem.indexOf(u8, path, "hkm") != null; +} + +fn printHelp() void { + prompt.intro("hkm uninstall"); + prompt.section("Usage"); + prompt.item("hkm uninstall", "remove every hkm install this user can remove"); + prompt.item("sudo hkm uninstall", "…including the system (.deb) install under /opt"); + prompt.item("hkm uninstall --dry-run", "show exactly what would go, remove nothing"); + prompt.blank(); + prompt.section("Removes"); + prompt.item("kernels", "/opt/hkm-kernel, ~/.local/lib/hkm-kernel, the pre-1.4 user kernel"); + prompt.item("launchers", "/usr/bin/{hkm,hkm-config}, ~/.local/bin/{hkm,hkm-config}"); + prompt.item("config", "~/.config/hkm (config.env)"); + prompt.item("cache", "the shared plugin store"); + prompt.item("package", "deregisters hkm-kernel from dpkg (remove, never purge)"); + prompt.blank(); + prompt.section("Never removes"); + prompt.item("your projects", "no path is read from the registry, cwd, or an argument"); + prompt.item("the registry", "projects.json + platform.json — rescued out of any kernel first"); + prompt.blank(); + prompt.section("Options"); + prompt.item("--dry-run, -n", "print the plan and exit"); + prompt.item("--yes, -y", "skip the confirmation prompt"); + prompt.item("--help, -h", "show this help"); + prompt.outro("reinstall later and `hkm list` still shows every project"); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test "removable rejects every catastrophic target" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + var env = EnvMap.init(a); + defer env.deinit(); + try env.put("HOME", "/home/tester"); + + // The ones that would be unrecoverable. + try std.testing.expect(!removable(&env, "/")); + try std.testing.expect(!removable(&env, "/opt")); + try std.testing.expect(!removable(&env, "/usr")); + try std.testing.expect(!removable(&env, "/home/tester")); // the home itself + try std.testing.expect(!removable(&env, "")); + try std.testing.expect(!removable(&env, "relative/path")); + // No "hkm" anywhere means it is not ours, whatever computed it. + try std.testing.expect(!removable(&env, "/home/tester/Documents")); + try std.testing.expect(!removable(&env, "/home/tester/.config")); +} + +test "removable accepts every real install path" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + var env = EnvMap.init(a); + defer env.deinit(); + try env.put("HOME", "/home/tester"); + + for ([_][]const u8{ + "/opt/hkm-kernel", + "/usr/bin/hkm", + "/usr/bin/hkm-config", + "/home/tester/.local/lib/hkm-kernel", + "/home/tester/.local/bin/hkm", + "/home/tester/.local/share/hkm/kernel", + "/home/tester/.config/hkm", + "/home/tester/.cache/hkm/plugin-store", + }) |p| { + try std.testing.expect(removable(&env, p)); + } +} + +test "the preserved registry directory is never itself a target" { + // The guarantee the whole command rests on. The userdata dir holds + // projects.json; the legacy KERNEL sits inside it and must still go, so the + // two paths are deliberately different and must not be confused. + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + var env = EnvMap.init(a); + defer env.deinit(); + try env.put("HOME", "/home/tester"); + + const keep = userdataDir(a, &env).?; + try std.testing.expectEqualStrings("/home/tester/.local/share/hkm", keep); + + const legacy = install_scope.legacyUserRoot(a, &env).?; + try std.testing.expectEqualStrings("/home/tester/.local/share/hkm/kernel", legacy); + + // The kernel is INSIDE the kept dir, and strictly deeper than it. + try std.testing.expect(!std.mem.eql(u8, keep, legacy)); + try std.testing.expect(util.isInside(legacy, keep)); +} + +test "userdata, config and cache directories match the writers' own resolution" { + // Each mirrors a different module (config.zig, userconfig.zig, + // plugin_store.zig). If one drifts, uninstall silently leaves that data + // behind — the failure mode is invisible, so it is pinned here. + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + var env = EnvMap.init(a); + defer env.deinit(); + try env.put("HOME", "/home/tester"); + + try std.testing.expectEqualStrings("/home/tester/.local/share/hkm", userdataDir(a, &env).?); + try std.testing.expectEqualStrings("/home/tester/.config/hkm", configDir(a, &env).?); + try std.testing.expectEqualStrings("/home/tester/.cache/hkm/plugin-store", pluginStore(a, &env).?); + + // XDG overrides win, exactly as the writers do it. + try env.put("XDG_CONFIG_HOME", "/cfg"); + try env.put("XDG_CACHE_HOME", "/cache"); + try env.put("XDG_DATA_HOME", "/data"); + try std.testing.expectEqualStrings("/data/hkm", userdataDir(a, &env).?); + try std.testing.expectEqualStrings("/cfg/hkm", configDir(a, &env).?); + try std.testing.expectEqualStrings("/cache/hkm/plugin-store", pluginStore(a, &env).?); + + // And an explicit userdata pin wins over XDG, as registry.zig reads it. + try env.put("HKM_USERDATA_DIR", "/srv/hkm-data"); + try std.testing.expectEqualStrings("/srv/hkm-data", userdataDir(a, &env).?); +} + +test "sudo preserves the invoking user's paths" { + // `sudo hkm uninstall` must remove the USER's install too — under sudo HOME + // is /root, so resolving from it would silently leave the user's kernel, + // config and cache on disk while reporting a complete uninstall. + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + var env = EnvMap.init(a); + defer env.deinit(); + try env.put("HOME", "/root"); + try env.put("SUDO_USER", "tester"); + + try std.testing.expectEqualStrings("/home/tester/.local/share/hkm", userdataDir(a, &env).?); + try std.testing.expectEqualStrings("/home/tester/.config/hkm", configDir(a, &env).?); + try std.testing.expectEqualStrings("/home/tester/.cache/hkm/plugin-store", pluginStore(a, &env).?); +} + +test "a registry inside a deletion target is detected, not deleted" { + // The critical case: HKM_USERDATA_DIR pointing into a kernel tree. The plan + // would list it under "Will KEEP" and delete its parent moments later — the + // exact loss this command promises cannot happen. isInside is what the + // guard keys on, so pin its behaviour for the real layouts. + try std.testing.expect(util.isInside("/opt/hkm-kernel/projects", "/opt/hkm-kernel")); + try std.testing.expect(util.isInside("/home/u/.local/lib/hkm-kernel/projects", "/home/u/.local/lib/hkm-kernel")); + + // …and the SAFE default must not trip the guard, or uninstall never runs. + try std.testing.expect(!util.isInside("/home/u/.local/share/hkm", "/home/u/.local/lib/hkm-kernel")); + try std.testing.expect(!util.isInside("/home/u/.local/share/hkm", "/opt/hkm-kernel")); + + // A sibling whose name merely starts the same way is not "inside". + try std.testing.expect(!util.isInside("/opt/hkm-kernel-old", "/opt/hkm-kernel")); +} diff --git a/tools/src/commands/upgrade.zig b/tools/src/commands/upgrade.zig index 4e16643..c6b3c1c 100644 --- a/tools/src/commands/upgrade.zig +++ b/tools/src/commands/upgrade.zig @@ -135,7 +135,27 @@ pub fn run(allocator: std.mem.Allocator, io: Io, env: *EnvMap, args: []const []c // `hkm upgrade` two different, predictable commands. var scope: ?Scope = null; + // Same reasoning as `uninstall`: an ignored typo here silently changes WHICH + // install is replaced. `--systm` would fall through to the privilege default + // and upgrade the user's install while the operator believed they had named + // the system one. + const known = [_][]const u8{ + "--check", "-c", "--local", "-l", "--dry-run", "-n", + "--yes", "-y", "--pre", "--user", "-u", "--system", + "-s", "--no-build", "--help", "-h", + }; + if (util.unknownFlag(args[1..], &known)) |bad| { + prompt.err(std.fmt.allocPrint(allocator, "unknown option: {s}", .{bad}) catch "unknown option"); + prompt.muted(" nothing was changed. Run `hkm upgrade --help` for the accepted flags."); + return 1; + } + for (args[1..]) |a| { + // Stop where unknownFlag stops. Without this the validator tolerated + // everything after a `--` while the parser below kept interpreting it, + // so `hkm upgrade -- --system` passed validation and then selected the + // system scope from an argument that was explicitly quoted out. + if (std.mem.eql(u8, a, "--")) break; if (std.mem.eql(u8, a, "--check") or std.mem.eql(u8, a, "-c")) check_only = true; if (std.mem.eql(u8, a, "--local") or std.mem.eql(u8, a, "-l")) from_local = true; if (std.mem.eql(u8, a, "--dry-run") or std.mem.eql(u8, a, "-n")) dry_run = true; @@ -455,15 +475,26 @@ fn linuxUpgrade( var argv = [_][]const u8{ "apt-get", "install", "-y", tmp }; const code = run_cmd.spawnWait(io, env, &argv) catch 1; if (code != 0) { - // Fallback: dpkg then fix deps. Both results are KEPT: with them - // discarded, an upgrade where apt AND dpkg both failed printed - // "updated" and left the old kernel installed — the user then - // debugs a version they believe they are no longer running. + // Fallback: dpkg, then repair dependencies, then dpkg AGAIN — + // and the verdict is that SECOND dpkg, never the repair. + // + // `apt-get -f install -y` exits 0 when it finds nothing to + // repair. So when `dpkg -i` failed for a reason that is not a + // missing dependency — a truncated download, a corrupt .deb — + // the repair returned 0 and the old condition + // (`dpkg_code != 0 and fix_code != 0`) was false. The command + // then printed "updated" with the previous kernel still + // installed: the exact outcome this block exists to prevent, + // and the same "upgrade did nothing" the rest of this release + // is about. Only a dpkg that succeeds proves the package landed. var dpkg = [_][]const u8{ "dpkg", "-i", tmp }; - const dpkg_code = run_cmd.spawnWait(io, env, &dpkg) catch 1; - var fix = [_][]const u8{ "apt-get", "-f", "install", "-y" }; - const fix_code = run_cmd.spawnWait(io, env, &fix) catch 1; - if (dpkg_code != 0 and fix_code != 0) { + var verdict = run_cmd.spawnWait(io, env, &dpkg) catch 1; + if (verdict != 0) { + var fix = [_][]const u8{ "apt-get", "-f", "install", "-y" }; + _ = run_cmd.spawnWait(io, env, &fix) catch {}; + verdict = run_cmd.spawnWait(io, env, &dpkg) catch 1; + } + if (verdict != 0) { prompt.err("installation FAILED — the previous kernel is still in place."); prompt.muted(try std.fmt.allocPrint(allocator, " the package is downloaded at {s}", .{tmp})); prompt.muted(" try it by hand: sudo apt-get install -y "); diff --git a/tools/src/commands/version.zig b/tools/src/commands/version.zig index a6c38b8..7ff5e89 100644 --- a/tools/src/commands/version.zig +++ b/tools/src/commands/version.zig @@ -224,17 +224,7 @@ fn warnings( } } -/// First match for `name` on PATH — the one a bare command actually runs. -fn findOnPath(allocator: std.mem.Allocator, io: Io, env: *EnvMap, name: []const u8) ?[]const u8 { - const path = env.get("PATH") orelse return null; - var it = std.mem.splitScalar(u8, path, ':'); - while (it.next()) |dir| { - if (dir.len == 0) continue; - const cand = std.fs.path.join(allocator, &.{ dir, name }) catch continue; - if (util.fileExists(io, cand)) return cand; - } - return null; -} +const findOnPath = util.findOnPath; fn printHelp() void { prompt.intro("hkm version"); @@ -245,8 +235,6 @@ fn printHelp() void { prompt.section("What the columns mean"); prompt.item("kernel version", "from /composer.json — the code that actually runs"); prompt.item("launcher", "the hkm binary for that scope, and the version it was built as"); - // Spelled out rather than printed as the bare glyph: prompt.item pads keys - // by byte length, and a 3-byte arrow would misalign the whole block. - prompt.item("arrow marker", "the install this invocation resolves"); + prompt.item("→", "the install this invocation resolves"); prompt.outro("a launcher and kernel that disagree is why an upgrade can look like a no-op"); } diff --git a/tools/src/config.zig b/tools/src/config.zig index 5cc2e05..508830c 100644 --- a/tools/src/config.zig +++ b/tools/src/config.zig @@ -43,6 +43,11 @@ pub fn main(init: std.process.Init.Minimal) !void { var env = try init.environ.createMap(allocator); defer env.deinit(); + // Bind the output streams before anything prints — same contract as the + // hkm launcher: results to stdout, diagnostics to stderr, colour only when + // the destination is a terminal. + prompt.init(io, &env); + // Make already-saved config visible to resolution below. userconfig.load(allocator, io, &env); @@ -58,7 +63,11 @@ pub fn main(init: std.process.Init.Minimal) !void { prompt.item("path", p); if (std.Io.Dir.cwd().readFileAlloc(io, p, allocator, .limited(64 * 1024))) |c| { prompt.blank(); - std.debug.print("{s}\n", .{c}); + // The whole point of `print` is to be captured, so the contents go + // to stdout. It rendered via std.debug.print (stderr), so + // `hkm-config print > cfg.env` produced an empty file — the same + // bug prompt.zig fixed for every other command. + prompt.raw(c); } else |_| prompt.muted("(file does not exist yet — run `hkm-config` to create it)"); return; } diff --git a/tools/src/lib/plugin_lock.zig b/tools/src/lib/plugin_lock.zig index 73a7654..37d3dcc 100644 --- a/tools/src/lib/plugin_lock.zig +++ b/tools/src/lib/plugin_lock.zig @@ -184,7 +184,9 @@ pub fn render(allocator: std.mem.Allocator, lock: *const Lock, kernelVersion: [] pub fn write(allocator: std.mem.Allocator, io: Io, projectRoot: []const u8, lock: *const Lock, kernelVersion: []const u8) !void { const body = try render(allocator, lock, kernelVersion); const p = try path(allocator, projectRoot); - try Dir.cwd().writeFile(io, .{ .sub_path = p, .data = body }); + // Atomic — plugins.lock.json is committed alongside composer.lock and a + // truncated one breaks `hkm plugins lock` for every checkout of the project. + try util.writeFileAtomic(io, p, body); } // --------------------------------------------------------------------------- diff --git a/tools/src/lib/prompt.zig b/tools/src/lib/prompt.zig index 3b1e6ed..9d6a4af 100644 --- a/tools/src/lib/prompt.zig +++ b/tools/src/lib/prompt.zig @@ -2,9 +2,76 @@ //! glyphs, colour accents, and styled intro / note / outro / error / prompt //! helpers. Text/confirm prompts read a line; `select` is an interactive //! raw-mode arrow-key list. The look matches the modern "prompts" experience. +//! +//! WHICH STREAM, AND WHY IT MATTERS +//! -------------------------------- +//! Results go to **stdout**; diagnostics and interactive prompts go to +//! **stderr**. Everything here used to render through `std.debug.print`, which +//! writes to stderr — so a command's OUTPUT was indistinguishable from its +//! errors, and the obvious thing a user tries produced an empty file: +//! +//! $ hkm list > projects.txt +//! $ wc -c projects.txt +//! 0 projects.txt +//! +//! That was found once before and fixed one function wide (`banner.printShort`, +//! whose docblock records it), but the cause is here, in the shared renderer. +//! The split now is the conventional one: `intro/section/item/ok/muted/note/ +//! table/outro` are the answer the caller asked for, while `err/warn` and every +//! prompt are commentary that must not pollute a pipe. +//! +//! COLOUR AND WIDTH ARE PROPERTIES OF THE DESTINATION +//! -------------------------------------------------- +//! ANSI was previously emitted unconditionally, so escape sequences landed in +//! log files and CI transcripts, and `NO_COLOR` did nothing. Both are now +//! decided per stream at `init`, from the same rule `tools/install.sh` has +//! always applied: no colour when `NO_COLOR` is set, when `TERM=dumb`, or when +//! that stream is not a terminal. const std = @import("std"); const Io = std.Io; +const EnvMap = std.process.Environ.Map; + +// ── destination state ─────────────────────────────────────────────────────── +// +// Process-wide, set once by main.zig / config.zig before any output. A file +// scope global is right here in a way it would not be in the kernel: this is a +// short-lived single-invocation CLI, and the alternative — threading an `Io` +// and an env map through all twenty rendering helpers and their several hundred +// call sites — buys nothing. + +var out_io: ?Io = null; +var color_out = false; +var color_err = false; +/// Terminal width of stdout, or null when stdout is not a terminal (do not +/// truncate — the consumer is a file or a pager, not an 80-column screen). +var out_cols: ?usize = null; + +/// Bind the streams and decide colour + width. Safe to call more than once. +/// +/// Before this runs, output falls back to `std.debug.print` (stderr, coloured), +/// which is what unit tests and any early-startup failure get. +pub fn init(io: Io, env: *EnvMap) void { + out_io = io; + + const no_color = blk: { + // Presence is what counts for NO_COLOR, not the value — that is the + // published convention (no-color.org), and honouring only "1" would + // ignore the common `NO_COLOR=` idiom people actually type. + if (env.get("NO_COLOR")) |v| break :blk v.len > 0; + break :blk false; + }; + const dumb = if (env.get("TERM")) |t| std.mem.eql(u8, t, "dumb") else false; + + const so = std.Io.File.stdout(); + const se = std.Io.File.stderr(); + const so_tty = so.isTty(io) catch false; + const se_tty = se.isTty(io) catch false; + + color_out = so_tty and !no_color and !dumb; + color_err = se_tty and !no_color and !dumb; + out_cols = if (so_tty) termCols() else null; +} // ── ANSI ────────────────────────────────────────────────────────────────── @@ -17,6 +84,101 @@ const yellow = "\x1b[33m"; const red = "\x1b[31m"; const gray = "\x1b[90m"; +/// Format once, then write to the chosen stream — stripping ANSI when that +/// stream is not receiving colour. +/// +/// Stripping at write time is deliberate: the styles above are concatenated +/// into the format strings at COMPILE time, so making colour conditional at +/// each of the ~40 call sites would mean rewriting every one of them into a +/// runtime branch. Removing the sequences on the way out gets the same result +/// from one place, and keeps the call sites readable. +fn emit(to_err: bool, comptime fmt: []const u8, args: anytype) void { + const io = out_io orelse { + std.debug.print(fmt, args); // pre-init: stderr, as before + return; + }; + + var buf: [8192]u8 = undefined; + const rendered = std.fmt.bufPrint(&buf, fmt, args) catch { + // Longer than the buffer (a pathological path). Fall back to a direct + // formatted write on the SAME stream — the earlier std.debug.print here + // silently rerouted oversized stdout lines to stderr, which is the very + // bug this module was changed to fix, reappearing only for long paths. + var w = streamFor(to_err).writerStreaming(io, &.{}); + w.interface.print(fmt, args) catch {}; + return; + }; + + const colored = if (to_err) color_err else color_out; + const file = streamFor(to_err); + + if (colored) { + file.writeStreamingAll(io, rendered) catch {}; + return; + } + + var plain: [8192]u8 = undefined; + file.writeStreamingAll(io, stripAnsi(rendered, &plain)) catch {}; +} + +fn streamFor(to_err: bool) std.Io.File { + return if (to_err) std.Io.File.stderr() else std.Io.File.stdout(); +} + +/// Write a block verbatim to stdout — no gutter, no styling. +/// +/// For content whose whole purpose is to be captured (a config file dumped by +/// `hkm-config print`), where the gutter would corrupt what the caller reads. +pub fn raw(block: []const u8) void { + const io = out_io orelse { + std.debug.print("{s}\n", .{block}); + return; + }; + std.Io.File.stdout().writeStreamingAll(io, block) catch {}; + std.Io.File.stdout().writeStreamingAll(io, "\n") catch {}; +} + +/// Copy `src` into `dst` with CSI escape sequences removed. +/// +/// Handles `ESC [ ... final`, where final is 0x40–0x7E — which covers every +/// sequence this module emits (colour, cursor-up, erase-line). `dst` is always +/// large enough because stripping only ever shortens. +fn stripAnsi(src: []const u8, dst: []u8) []const u8 { + var w: usize = 0; + var i: usize = 0; + while (i < src.len) { + // An ESC always begins something that must not reach a log file, so it + // is dropped whether or not a complete sequence follows. emit() renders + // into a fixed buffer, which means a sequence CAN be cut in half at the + // end of input — and the earlier form, which only skipped on a complete + // "ESC [", copied that dangling ESC straight through. + if (src[i] == 0x1b) { + i += 1; + if (i < src.len and src[i] == '[') { + i += 1; + while (i < src.len and !(src[i] >= 0x40 and src[i] <= 0x7E)) i += 1; + if (i < src.len) i += 1; // consume the final byte + } + continue; + } + if (w >= dst.len) break; + dst[w] = src[i]; + w += 1; + i += 1; + } + return dst[0..w]; +} + +/// Results — stdout. +fn out(comptime fmt: []const u8, args: anytype) void { + emit(false, fmt, args); +} + +/// Diagnostics and prompts — stderr. +fn diag(comptime fmt: []const u8, args: anytype) void { + emit(true, fmt, args); +} + // ── glyphs ────────────────────────────────────────────────────────────────── const bar = dim ++ "│" ++ reset; @@ -27,66 +189,102 @@ const corner_bot = green ++ "└" ++ reset; /// Opening banner: `┌ ` then a gutter line. pub fn intro(title: []const u8) void { - std.debug.print("\n" ++ corner_top ++ " " ++ bold ++ "{s}" ++ reset ++ "\n" ++ bar ++ "\n", .{title}); + out("\n" ++ corner_top ++ " " ++ bold ++ "{s}" ++ reset ++ "\n" ++ bar ++ "\n", .{title}); } /// Closing banner: a gutter line then `└ <message>` in green. pub fn outro(message: []const u8) void { - std.debug.print(bar ++ "\n" ++ corner_bot ++ " " ++ green ++ "{s}" ++ reset ++ "\n\n", .{message}); + out(bar ++ "\n" ++ corner_bot ++ " " ++ green ++ "{s}" ++ reset ++ "\n\n", .{message}); } /// A plain line under the gutter. pub fn note(line: []const u8) void { - std.debug.print(bar ++ " {s}\n", .{line}); + out(bar ++ " {s}\n", .{line}); } /// A success note (green check). pub fn ok(line: []const u8) void { - std.debug.print(bar ++ " " ++ green ++ "✓" ++ reset ++ " {s}\n", .{line}); + out(bar ++ " " ++ green ++ "✓" ++ reset ++ " {s}\n", .{line}); } /// An informational/secondary note (dimmed). pub fn muted(line: []const u8) void { - std.debug.print(bar ++ " " ++ gray ++ "{s}" ++ reset ++ "\n", .{line}); + out(bar ++ " " ++ gray ++ "{s}" ++ reset ++ "\n", .{line}); } -/// A warning note (yellow). +/// A warning note (yellow) — stderr: commentary, not the answer. pub fn warn(line: []const u8) void { - std.debug.print(bar ++ " " ++ yellow ++ "▲ {s}" ++ reset ++ "\n", .{line}); + diag(bar ++ " " ++ yellow ++ "▲ {s}" ++ reset ++ "\n", .{line}); } /// An empty gutter line — vertical spacing inside a help/prompt block. pub fn blank() void { - std.debug.print(bar ++ "\n", .{}); + out(bar ++ "\n", .{}); } /// A bold section heading under the gutter (e.g. "Usage", "Options"). pub fn section(title: []const u8) void { - std.debug.print(bar ++ " " ++ bold ++ "{s}" ++ reset ++ "\n", .{title}); + out(bar ++ " " ++ bold ++ "{s}" ++ reset ++ "\n", .{title}); } -/// A two-column help row: a cyan key padded to 30 cols, then a dimmed +/// A two-column help row: a cyan key padded to 30 columns, then a dimmed /// description. Use for usage lines, flags, env vars, and examples. pub fn item(key: []const u8, desc: []const u8) void { - // A key longer than the column still needs a gap before its description. + // Padding is measured in DISPLAY columns, not bytes. + // + // This used to use `key.len` and `{s: <30}`, both of which count bytes — so + // a key containing any multi-byte glyph consumed its byte length in padding + // while occupying one column, and the description column shifted left. A + // single "→" key (3 bytes) misaligned the row by two. displayWidth() below + // was already written for table(); item() simply never used it. + const w = displayWidth(key); + + // A key at or past the column still needs a gap before its description. // Without one, every long usage line in `--help` read as one run-on word: // "hkm plugins enable <plugin> [proj]wire a plugin into the project". - if (key.len >= 30) { - std.debug.print( + if (w >= 30) { + out( bar ++ " " ++ cyan ++ "{s}" ++ reset ++ " " ++ gray ++ "{s}" ++ reset ++ "\n", .{ key, desc }, ); return; } - std.debug.print( - bar ++ " " ++ cyan ++ "{s: <30}" ++ reset ++ gray ++ "{s}" ++ reset ++ "\n", - .{ key, desc }, + + var pad_buf: [30]u8 = undefined; + const pad = pad_buf[0 .. 30 - w]; + @memset(pad, ' '); + out( + bar ++ " " ++ cyan ++ "{s}{s}" ++ reset ++ gray ++ "{s}" ++ reset ++ "\n", + .{ key, pad, desc }, ); } -/// A standalone error block (red), for fatal failures. +/// A remediation line that belongs with a preceding `err` / `warn` — stderr. +/// +/// `item` and `muted` render results, so they go to stdout. The lines that +/// FOLLOW a diagnostic are part of that diagnostic ("here is how to fix it"), +/// and sending them to stdout split a two-line message across two streams: the +/// error appeared in the terminal while its fix landed in the redirected file. +pub fn hint(key: []const u8, desc: []const u8) void { + const w = displayWidth(key); + if (w >= 30) { + diag(bar ++ " " ++ cyan ++ "{s}" ++ reset ++ " " ++ gray ++ "{s}" ++ reset ++ "\n", .{ key, desc }); + return; + } + var pad_buf: [30]u8 = undefined; + const pad = pad_buf[0 .. 30 - w]; + @memset(pad, ' '); + diag(bar ++ " " ++ cyan ++ "{s}{s}" ++ reset ++ gray ++ "{s}" ++ reset ++ "\n", .{ key, pad, desc }); +} + +/// A dimmed follow-up line for a diagnostic — stderr, for the same reason. +pub fn hintLine(line: []const u8) void { + diag(bar ++ " " ++ gray ++ "{s}" ++ reset ++ "\n", .{line}); +} + +/// A standalone error block (red), for fatal failures — always stderr. pub fn err(message: []const u8) void { - std.debug.print("\n" ++ red ++ "■ {s}" ++ reset ++ "\n\n", .{message}); + diag("\n" ++ red ++ "■ {s}" ++ reset ++ "\n\n", .{message}); } /// Free-text prompt. Renders `◆ <label> [default]`, reads a line on the gutter, @@ -94,15 +292,15 @@ pub fn err(message: []const u8) void { /// always heap-duped so the caller owns it. pub fn text(allocator: std.mem.Allocator, io: Io, label: []const u8, default: []const u8) ![]const u8 { if (default.len > 0) { - std.debug.print(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ " " ++ dim ++ "[{s}]" ++ reset ++ "\n", .{ label, default }); + diag(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ " " ++ dim ++ "[{s}]" ++ reset ++ "\n", .{ label, default }); } else { - std.debug.print(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ "\n", .{label}); + diag(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ "\n", .{label}); } - std.debug.print(bar ++ " " ++ cyan, .{}); + diag(bar ++ " " ++ cyan, .{}); var buf: [4096]u8 = undefined; const line = readLine(io, &buf); - std.debug.print(reset ++ bar ++ "\n", .{}); + diag(reset ++ bar ++ "\n", .{}); const trimmed = std.mem.trim(u8, line, " \t\r\n"); return allocator.dupe(u8, if (trimmed.len == 0) default else trimmed); @@ -111,13 +309,13 @@ pub fn text(allocator: std.mem.Allocator, io: Io, label: []const u8, default: [] /// Yes/No prompt. Renders `◆ <label> [Y/n]` (or `[y/N]`) and parses the answer, /// falling back to `default_yes` on empty/unrecognised input. pub fn confirm(io: Io, label: []const u8, default_yes: bool) bool { - const hint = if (default_yes) "[Y/n]" else "[y/N]"; - std.debug.print(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ " " ++ dim ++ "{s}" ++ reset ++ "\n", .{ label, hint }); - std.debug.print(bar ++ " " ++ cyan, .{}); + const suffix = if (default_yes) "[Y/n]" else "[y/N]"; + diag(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ " " ++ dim ++ "{s}" ++ reset ++ "\n", .{ label, suffix }); + diag(bar ++ " " ++ cyan, .{}); var buf: [64]u8 = undefined; const line = readLine(io, &buf); - std.debug.print(reset ++ bar ++ "\n", .{}); + diag(reset ++ bar ++ "\n", .{}); const t = std.mem.trim(u8, line, " \t\r\n"); if (t.len == 0) return default_yes; @@ -136,15 +334,15 @@ pub fn select(label: []const u8, items: []const []const u8) ?usize { const tty = std.posix.STDIN_FILENO; const orig = std.posix.tcgetattr(tty) catch return 0; // not a TTY → first item - var raw = orig; - raw.lflag.ICANON = false; - raw.lflag.ECHO = false; - raw.lflag.ISIG = false; - raw.lflag.IEXTEN = false; - std.posix.tcsetattr(tty, .NOW, raw) catch {}; + var raw_mode = orig; + raw_mode.lflag.ICANON = false; + raw_mode.lflag.ECHO = false; + raw_mode.lflag.ISIG = false; + raw_mode.lflag.IEXTEN = false; + std.posix.tcsetattr(tty, .NOW, raw_mode) catch {}; defer std.posix.tcsetattr(tty, .NOW, orig) catch {}; - std.debug.print(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ "\n", .{label}); + diag(diamond_active ++ " " ++ bold ++ "{s}" ++ reset ++ "\n", .{label}); drawOptions(items, 0); var cur: usize = 0; @@ -163,13 +361,13 @@ pub fn select(label: []const u8, items: []const []const u8) ?usize { } else switch (buf[0]) { 'k' => { cur = if (cur == 0) items.len - 1 else cur - 1; moved = true; }, 'j' => { cur = (cur + 1) % items.len; moved = true; }, - '\r', '\n' => { std.debug.print(bar ++ "\n", .{}); return cur; }, + '\r', '\n' => { diag(bar ++ "\n", .{}); return cur; }, 'q', 0x1b, 3, 4 => return null, // q / Esc / Ctrl+C / Ctrl+D else => {}, } if (moved) { - std.debug.print("\x1b[{d}A", .{items.len}); // cursor up to redraw in place + diag("\x1b[{d}A", .{items.len}); // cursor up to redraw in place drawOptions(items, cur); } } @@ -180,9 +378,9 @@ pub fn select(label: []const u8, items: []const []const u8) ?usize { fn drawOptions(items: []const []const u8, cur: usize) void { for (items, 0..) |it, i| { if (i == cur) { - std.debug.print("\x1b[2K" ++ bar ++ " " ++ cyan ++ "❯ " ++ bold ++ "{s}" ++ reset ++ "\n", .{it}); + diag("\x1b[2K" ++ bar ++ " " ++ cyan ++ "❯ " ++ bold ++ "{s}" ++ reset ++ "\n", .{it}); } else { - std.debug.print("\x1b[2K" ++ bar ++ " " ++ dim ++ "{s}" ++ reset ++ "\n", .{it}); + diag("\x1b[2K" ++ bar ++ " " ++ dim ++ "{s}" ++ reset ++ "\n", .{it}); } } } @@ -239,10 +437,18 @@ fn renderTable( // 2. Shrink to fit: budget = terminal width minus the gutter and the box // overhead ((ncol+1) borders + 2 padding spaces per column). - const cols = termCols(); - const overhead = (ncol + 1) + 2 * ncol; - const avail = if (cols > gutter_cols + overhead) cols - gutter_cols - overhead else 0; - if (avail > 0) shrinkToFit(widths, avail); + // + // ONLY when stdout is a terminal. termCols() falls back to 80 whenever + // the ioctl fails, which is exactly the redirected case — so piping used + // to truncate every long path to fit a screen that was not there, and + // the `…` was the only sign anything had been dropped. Trimming to fit a + // terminal is right; trimming to fit an IMAGINED one loses data the + // consumer (a file, a pager, another program) would have shown in full. + if (out_cols) |cols| { + const overhead = (ncol + 1) + 2 * ncol; + const avail = if (cols > gutter_cols + overhead) cols - gutter_cols - overhead else 0; + if (avail > 0) shrinkToFit(widths, avail); + } // 3. Emit. A scratch buffer is reused for every line. var line: std.ArrayList(u8) = .empty; @@ -348,7 +554,7 @@ fn appendRepeat(allocator: std.mem.Allocator, line: *std.ArrayList(u8), glyph: [ } fn flush(line: *std.ArrayList(u8)) void { - std.debug.print("{s}\n", .{line.items}); + out("{s}\n", .{line.items}); } /// Display width in terminal columns: UTF-8 scalar count (continuation bytes — @@ -370,3 +576,57 @@ fn termCols() usize { if (signed >= 0 and ws.col > 0) return ws.col; return 80; } + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test "stripAnsi removes every sequence this module emits" { + // Colour, cursor-up and erase-line all appear in the format strings above. + // A sequence that survives stripping lands as mojibake in a log file, which + // is the whole reason non-TTY output is stripped at all. + var buf: [256]u8 = undefined; + + try std.testing.expectEqualStrings("plain", stripAnsi("plain", &buf)); + try std.testing.expectEqualStrings("hi", stripAnsi(cyan ++ "hi" ++ reset, &buf)); + try std.testing.expectEqualStrings("│ ok", stripAnsi(bar ++ " " ++ green ++ "ok" ++ reset, &buf)); + try std.testing.expectEqualStrings("x", stripAnsi("\x1b[2K" ++ "x", &buf)); // erase-line + try std.testing.expectEqualStrings("", stripAnsi("\x1b[12A", &buf)); // cursor-up, multi-digit +} + +test "stripAnsi keeps non-ASCII text intact" { + // The gutter, arrows and box-drawing characters are all multi-byte UTF-8 and + // must survive — stripping targets escape sequences, not high bytes. + var buf: [256]u8 = undefined; + try std.testing.expectEqualStrings("→ café ✓ ┌", stripAnsi("→ café ✓ ┌", &buf)); + try std.testing.expectEqualStrings("▲ warn", stripAnsi(yellow ++ "▲ warn" ++ reset, &buf)); +} + +test "stripAnsi tolerates a truncated escape at the end of input" { + // emit() renders into a fixed buffer, so a sequence can be cut mid-way. That + // must not read past the slice. + var buf: [64]u8 = undefined; + try std.testing.expectEqualStrings("a", stripAnsi("a\x1b[", &buf)); + try std.testing.expectEqualStrings("a", stripAnsi("a\x1b", &buf)); + try std.testing.expectEqualStrings("a", stripAnsi("a\x1b[3", &buf)); +} + +test "displayWidth counts columns, not bytes" { + // The bug behind P7: item() padded with key.len, so a 3-byte glyph consumed + // three columns of padding while occupying one. + try std.testing.expectEqual(@as(usize, 5), displayWidth("plain")); + try std.testing.expectEqual(@as(usize, 1), displayWidth("→")); // 3 bytes + try std.testing.expectEqual(@as(usize, 1), displayWidth("│")); // 3 bytes + try std.testing.expectEqual(@as(usize, 4), displayWidth("café")); // 5 bytes + try std.testing.expect(displayWidth("→") != "→".len); +} + +test "item pads to a constant column for ASCII and non-ASCII alike" { + // Reproduces the alignment property directly: whatever the key, the + // description starts at the same column. Computed the way item() does it. + for ([_][]const u8{ "launcher", "→", "café", "kernel version" }) |key| { + const w = displayWidth(key); + try std.testing.expect(w < 30); + try std.testing.expectEqual(@as(usize, 30), w + (30 - w)); + } +} diff --git a/tools/src/lib/registry.zig b/tools/src/lib/registry.zig index c1f823a..8bc7bf1 100644 --- a/tools/src/lib/registry.zig +++ b/tools/src/lib/registry.zig @@ -129,7 +129,9 @@ pub fn upsert(allocator: std.mem.Allocator, io: Io, jsonPath: []const u8, entry: if (!replaced) try entries.append(allocator, entry); const out = try render(allocator, entries.items); - try cwd.writeFile(io, .{ .sub_path = jsonPath, .data = out }); + // Atomic: this is the project registry, and a half-written one loses every + // registration on the machine. See util.writeFileAtomic. + try util.writeFileAtomic(io, jsonPath, out); } /// Parse the registry JSON object into `entries`. diff --git a/tools/src/lib/util.zig b/tools/src/lib/util.zig index 8b8b580..cf8c710 100644 --- a/tools/src/lib/util.zig +++ b/tools/src/lib/util.zig @@ -51,6 +51,118 @@ pub fn chmod600(io: Io, path: []const u8) void { f.setPermissions(io, @enumFromInt(0o600)) catch {}; } +/// Locate an executable by walking PATH, returning the FIRST match — the one +/// that would actually run. Null when it is nowhere on PATH. +/// +/// A name containing a '/' is treated as a path, not a PATH lookup, matching +/// what execve itself does — so `HKM_PHP_BIN=/opt/php/bin/php` resolves to that +/// file rather than being searched for as a filename. +/// +/// Lives here because `doctor` and `version` had grown a private copy each, and +/// a third was about to appear in main.zig for the passthrough diagnostic. +pub fn findOnPath(allocator: std.mem.Allocator, io: Io, env: *EnvMap, name: []const u8) ?[]const u8 { + if (name.len == 0) return null; + + if (std.mem.indexOfScalar(u8, name, '/') != null) { + return if (fileExists(io, name)) name else null; + } + + const path = env.get("PATH") orelse return null; + var it = std.mem.splitScalar(u8, path, ':'); + while (it.next()) |entry| { + // POSIX: an EMPTY PATH element means the current directory. Skipping it + // made findOnPath disagree with the shell that resolved the command. + const dir = if (entry.len == 0) "." else entry; + const cand = std.fs.path.join(allocator, &.{ dir, name }) catch continue; + if (fileExists(io, cand)) return cand; + } + return null; +} + +/// Whether an executable is runnable — `findOnPath` as a predicate. +pub fn onPath(allocator: std.mem.Allocator, io: Io, env: *EnvMap, name: []const u8) bool { + return findOnPath(allocator, io, env, name) != null; +} + +/// Every match for `name` on PATH, counted — used to detect one install +/// shadowing another. +pub fn countOnPath(allocator: std.mem.Allocator, io: Io, env: *EnvMap, name: []const u8) usize { + const path = env.get("PATH") orelse return 0; + var n: usize = 0; + var it = std.mem.splitScalar(u8, path, ':'); + while (it.next()) |dir| { + if (dir.len == 0) continue; + const cand = std.fs.path.join(allocator, &.{ dir, name }) catch continue; + if (fileExists(io, cand)) n += 1; + } + return n; +} + +/// The first argument that looks like a flag but is not in `known`, or null. +/// +/// Every command in this tree parses flags by testing for the ones it knows and +/// ignoring the rest, with a comment saying that keeps future flags from hard +/// failing. For a read-only command that is a fair trade. For a DESTRUCTIVE one +/// it is not, because the ignored token is usually a typo of the flag that was +/// meant to make it safe: +/// +/// hkm uninstall --dryrun --yes # --dry-run misspelled +/// +/// which parsed as "no dry run, and don't ask" and deleted the install. Commands +/// that can destroy or escalate should reject what they do not recognise. +/// +/// `--flag=value` is compared on the name before `=`. A bare `-` or `--` is not +/// treated as a flag, and everything after a `--` separator is left alone so a +/// passthrough command can forward its own arguments. +pub fn unknownFlag(args: []const []const u8, known: []const []const u8) ?[]const u8 { + for (args) |raw| { + if (std.mem.eql(u8, raw, "--")) return null; // end of options + if (raw.len < 2 or raw[0] != '-') continue; // positional + if (std.mem.eql(u8, raw, "-")) continue; // stdin convention + + const name = if (std.mem.indexOfScalar(u8, raw, '=')) |i| raw[0..i] else raw; + if (!contains(known, name)) return raw; + } + return null; +} + +/// Write `data` to `path` atomically: fill a sibling temp file, then rename it +/// over the target. +/// +/// `writeFile` truncates first and then writes, so a process killed part way +/// through — or a full disk — leaves a truncated file rather than the old one. +/// That is tolerable for a cache and not for USER DATA: `projects.json` is the +/// registry of every project on the machine, the one file `hkm uninstall` +/// deliberately rescues before deleting anything, and it was being replaced by +/// the destructive method. +/// +/// rename(2) within a directory is atomic, so a reader sees either the previous +/// file or the new one and never a half-written one. The pattern is already used +/// twice in this tree — `install.sh` stages to `.new` before `mv`, and the +/// launcher install writes `.hkm-new` then renames — it had simply never reached +/// the registry. +pub fn writeFileAtomic(io: Io, path: []const u8, data: []const u8) !void { + // The temp name carries the pid, so two hkm processes writing the same + // registry cannot land on one another's staging file — one would otherwise + // rename a half-written copy over the target the other was still filling. + const pid: u32 = switch (@import("builtin").os.tag) { + .windows => 0, + .linux => @bitCast(std.os.linux.getpid()), + else => @bitCast(std.c.getpid()), + }; + var buf: [std.fs.max_path_bytes]u8 = undefined; + const tmp = std.fmt.bufPrint(&buf, "{s}.hkm-tmp.{d}", .{ path, pid }) catch { + // No room for the suffix — a direct write still beats not writing. + return Dir.cwd().writeFile(io, .{ .sub_path = path, .data = data }); + }; + + try Dir.cwd().writeFile(io, .{ .sub_path = tmp, .data = data }); + Dir.cwd().rename(tmp, Dir.cwd(), path, io) catch |e| { + Dir.cwd().deleteFile(io, tmp) catch {}; + return e; + }; +} + /// Is `path` a symbolic link? readLink succeeds only on one. pub fn isSymlink(io: Io, path: []const u8) bool { var buf: [std.fs.max_path_bytes]u8 = undefined; @@ -322,3 +434,37 @@ test "an existing suffix is not doubled" { // Not a suffix, merely a substring. try std.testing.expectEqualStrings("SeederThing", stripSuffix("SeederThing", "Seeder")); } + +test "unknownFlag catches the typo that made a destructive command run for real" { + // The exact input that deleted an install: `--dry-run` misspelled, so the + // guard flag was ignored and `--yes` suppressed the confirmation. + const known = [_][]const u8{ "--dry-run", "-n", "--yes", "-y", "--help", "-h" }; + const args = [_][]const u8{ "--dryrun", "--yes" }; + try std.testing.expectEqualStrings("--dryrun", unknownFlag(&args, &known).?); +} + +test "unknownFlag accepts every flag a command declares" { + const known = [_][]const u8{ "--dry-run", "-n", "--yes", "-y" }; + try std.testing.expect(unknownFlag(&[_][]const u8{ "--dry-run", "-y" }, &known) == null); + try std.testing.expect(unknownFlag(&[_][]const u8{}, &known) == null); +} + +test "unknownFlag ignores positionals and the stdin dash" { + const known = [_][]const u8{"--yes"}; + try std.testing.expect(unknownFlag(&[_][]const u8{ "shop", "--yes" }, &known) == null); + try std.testing.expect(unknownFlag(&[_][]const u8{"-"}, &known) == null); + // A negative number is a positional, not a flag people expect to be rejected. + try std.testing.expect(unknownFlag(&[_][]const u8{"/some/path"}, &known) == null); +} + +test "unknownFlag compares --flag=value on the name" { + const known = [_][]const u8{"--prefix"}; + try std.testing.expect(unknownFlag(&[_][]const u8{"--prefix=/srv/hkm"}, &known) == null); + try std.testing.expectEqualStrings("--prefx=/srv", unknownFlag(&[_][]const u8{"--prefx=/srv"}, &known).?); +} + +test "unknownFlag stops at a -- separator so passthrough args are left alone" { + const known = [_][]const u8{"--yes"}; + const args = [_][]const u8{ "--yes", "--", "--anything-goes", "-x" }; + try std.testing.expect(unknownFlag(&args, &known) == null); +} diff --git a/tools/src/main.zig b/tools/src/main.zig index 6439d39..3bbec1a 100644 --- a/tools/src/main.zig +++ b/tools/src/main.zig @@ -10,6 +10,7 @@ const ui_cmd = @import("commands/ui.zig"); const cli_cmd = @import("commands/cli.zig"); const doctor_cmd = @import("commands/doctor.zig"); const upgrade_cmd = @import("commands/upgrade.zig"); +const uninstall_cmd = @import("commands/uninstall.zig"); const version_cmd = @import("commands/version.zig"); const kernel = @import("lib/kernel.zig"); const util = @import("lib/util.zig"); @@ -34,6 +35,7 @@ fn printHelp(allocator: std.mem.Allocator, io: std.Io, env: *std.process.Environ prompt.item("hkm update <path|name>", "refresh a project's kernel registry entry"); prompt.item("hkm upgrade [--check]", "update YOUR install; sudo hkm upgrade updates the system one"); prompt.item("hkm upgrade --local", "install THIS checkout over an installed kernel"); + prompt.item("hkm uninstall", "remove every hkm install (keeps projects + registry)"); prompt.item("hkm doctor", "diagnose the local environment"); prompt.item("hkm version", "kernel version in each install scope (also --version, -v)"); prompt.item("hkm help", "show this help"); @@ -61,6 +63,64 @@ fn envGet(allocator: std.mem.Allocator, map: *std.process.Environ.Map, key: []co return try allocator.dupe(u8, v); } +/// Explain why the PHP passthrough could not start. +/// +/// Everything not handled natively in Zig is forwarded to the kernel's PHP CLI, +/// so this one spawn is where a typo and a missing runtime both surface. It has +/// to separate them, because the fixes are unrelated: one is "you meant a +/// different word", the other is "this machine has no PHP". +fn reportPassthroughFailure( + allocator: std.mem.Allocator, + io: std.Io, + env: *std.process.Environ.Map, + e: anyerror, + php: []const u8, + cli: []const u8, + cmd: []const u8, +) u8 { + // Order matters: spawn() launches PHP with `cli` as its ARGUMENT, so a + // missing `cli` is not a PHP problem. Checking php first would have blamed + // the interpreter for a kernel that simply is not installed. + const have_php = util.onPath(allocator, io, env, php); + const have_cli = util.fileExists(io, cli); + + if (have_php and !have_cli) { + prompt.err(std.fmt.allocPrint( + allocator, + "the kernel's PHP CLI is missing: {s}", + .{cli}, + ) catch "the kernel's PHP CLI is missing."); + prompt.item("diagnose it", "hkm doctor"); + prompt.item("show installs", "hkm version"); + prompt.item("reinstall", "hkm upgrade"); + return 1; + } + + if (!have_php) { + prompt.err(std.fmt.allocPrint( + allocator, + "PHP is required to run `hkm {s}`, and `{s}` was not found on your PATH.", + .{ cmd, php }, + ) catch "PHP was not found on your PATH."); + prompt.item("see what is missing", "hkm doctor"); + prompt.item("Debian/Ubuntu", "sudo apt install php8.4-cli"); + prompt.item("macOS", "brew install php"); + prompt.item("or point at it", "HKM_PHP_BIN=/full/path/to/php"); + return 1; + } + + // PHP and the CLI both exist, so the command word itself is the suspect — + // `hkm` forwards anything it does not handle, and a typo lands here. + prompt.err(std.fmt.allocPrint( + allocator, + "could not run `hkm {s}` ({t}).", + .{ cmd, e }, + ) catch "could not run that command."); + prompt.item("list the commands", "hkm help"); + prompt.item("check the environment", "hkm doctor"); + return 1; +} + fn findCliPath(allocator: std.mem.Allocator, io: std.Io, env_map: *std.process.Environ.Map) ![]const u8 { return kernel.findCliPath(allocator, io, env_map); } @@ -171,6 +231,11 @@ fn dispatch(init: std.process.Init.Minimal, mm: *memory.Manager) !u8 { var env_map = try init.environ.createMap(allocator); defer env_map.deinit(); + // Bind the output streams BEFORE anything can print. This decides results → + // stdout / diagnostics → stderr, and whether either gets colour. Until it + // runs, prompt falls back to stderr, so it must come first. + prompt.init(io, &env_map); + // Load persistent config (~/.config/hkm/config.env) so values written by // `hkm-config` take effect. Real environment variables always win. userconfig.load(allocator, io, &env_map); @@ -263,6 +328,11 @@ fn dispatch(init: std.process.Init.Minimal, mm: *memory.Manager) !u8 { defer scope.end(); return try upgrade_cmd.run(scope.allocator(), io, &env_map, args); } + if (std.mem.eql(u8, cmd, "uninstall")) { + var scope = CmdScope.begin(mm, "uninstall"); + defer scope.end(); + return try uninstall_cmd.run(scope.allocator(), io, &env_map, args); + } // `new` / `update` are handled natively in Zig (no PHP required). if (std.mem.eql(u8, cmd, "new")) { @@ -339,13 +409,21 @@ fn dispatch(init: std.process.Init.Minimal, mm: *memory.Manager) !u8 { try child_argv.append(pass, args[i]); } - var child = try std.process.spawn(io, .{ + // A spawn failure here is the ONLY thing standing between a mistyped + // command and a raw Zig error as the user-facing message. Propagating it + // printed exactly `error: FileNotFound` on a release build — no filename, + // no mention of PHP, and no pointer to `hkm doctor`, the command that + // exists to explain a missing runtime. Both causes are known at this point, + // so both get said. + var child = std.process.spawn(io, .{ .argv = child_argv.items, .environ_map = &env_map, .stdin = .inherit, .stdout = .inherit, .stderr = .inherit, - }); + }) catch |e| { + return reportPassthroughFailure(pass, io, &env_map, e, php, cli, cmd); + }; const term = try child.wait(io); return switch (term) { diff --git a/tools/src/tests.zig b/tools/src/tests.zig index 2a02c07..6278dcc 100644 --- a/tools/src/tests.zig +++ b/tools/src/tests.zig @@ -34,6 +34,7 @@ test { _ = @import("commands/plugins.zig"); _ = @import("commands/run.zig"); _ = @import("commands/ui.zig"); + _ = @import("commands/uninstall.zig"); _ = @import("commands/update.zig"); _ = @import("commands/upgrade.zig"); _ = @import("commands/version.zig");