diff --git a/changelog.d/upgrade-apply-mode.added.md b/changelog.d/upgrade-apply-mode.added.md new file mode 100644 index 0000000000..64da3ee2f6 --- /dev/null +++ b/changelog.d/upgrade-apply-mode.added.md @@ -0,0 +1 @@ +- `wheels upgrade apply` performs the framework swap: it replaces the app's `vendor/wheels/` with the framework bundled in the installed CLI, announcing the exact backup destination (`vendor/wheels.bak-/`) and the one-line recovery command before touching anything (`--nobackup` opts out; a mid-copy failure throws `Wheels.FrameworkUpgrader.CopyFailed` naming the backup to restore from). Bare `wheels upgrade` prints usage and never modifies files — destructive commands require the explicit verb, so MCP clients calling `wheels_upgrade` with `{}` can never trigger the swap. Safety rails fire before any mutation: refuses outside a Wheels app, when source or target doesn't sniff as a real framework directory (`wheels.json`/`box.json` must carry a non-empty version identifying Wheels — a generic app `box.json` is rejected), when source and target resolve to the same directory (e.g. inside the wheels repo checkout), on unknown flags/subcommands, and when `--to=` doesn't match the bundled framework version — downloading arbitrary `--to=` targets is the planned follow-up. `wheels upgrade check` keeps the read-only scan unchanged (including `--strict`, `--format=json`, and the non-zero-exit contract); its closing hint now points at `wheels upgrade apply` instead of `brew upgrade wheels`, which only ever upgraded the CLI binary ([#3035](https://github.com/wheels-dev/wheels/issues/3035)) diff --git a/cli/lucli/Module.cfc b/cli/lucli/Module.cfc index 3d9a863ee2..5e3f72f95a 100644 --- a/cli/lucli/Module.cfc +++ b/cli/lucli/Module.cfc @@ -306,10 +306,11 @@ component extends="modules.BaseModule" { private any function upgradeArgSpec() { return new services.ArgSpec() - .positional(name = "subcommand", default = "", description = "Only `check` is supported — scans the app for breaking changes (read-only)") - .option(name = "to", default = "", description = "Target Wheels version to check against (defaults to latest)") - .option(name = "format", default = "", description = "Set to json for machine-readable output") - .flag(name = "strict", default = false, description = "Escalate advisory findings to a hard failure (non-zero exit) so CI can gate on them"); + .positional(name = "subcommand", default = "", description = "Explicit verb required: `check` scans for breaking changes (read-only); `apply` swaps vendor/wheels/ with the CLI's bundled framework (backup first). Omitted/empty prints usage and never modifies files") + .option(name = "to", default = "", description = "Target Wheels version. check: version to scan against (default: latest). apply: must match the CLI's bundled framework version") + .option(name = "format", default = "", description = "check only: set to json for machine-readable output") + .flag(name = "strict", default = false, description = "check only: escalate advisory findings to a hard failure (non-zero exit) so CI can gate on them") + .flag(name = "nobackup", default = false, description = "apply only: skip the vendor/wheels.bak- backup of the existing framework"); } // ───────────────────────────────────────────────── @@ -443,7 +444,7 @@ component extends="modules.BaseModule" { help &= " notes Find TODO / FIXME / OPTIMIZE comments (--annotations to customize)" & nl & nl; help &= "Packages & Deployment:" & nl; help &= " packages Add, update, search Wheels packages (verb is `add`, not `install`)" & nl; - help &= " upgrade Scan for breaking changes before upgrading Wheels (read-only)" & nl; + help &= " upgrade Upgrade the Wheels framework in your app (vendor/wheels/); `check` scans, `apply` swaps" & nl; help &= " deploy Deploy your app (Kamal-compatible)" & nl & nl; help &= "Other:" & nl; help &= " mcp Configure Wheels MCP server for AI assistants" & nl; @@ -2833,15 +2834,43 @@ component extends="modules.BaseModule" { // ───────────────────────────────────────────────── /** - * Parse `wheels upgrade` arguments. `subcommand` (positional) must be - * "check"; `--to=` selects the target. `sawTo` / `sawDryRun` drive - * the "did you mean" nudge and match both `--to` and `--to=x` (LuCLI maps a - * bare `--to` to to=true and `--to=x` to to=x — either way the key exists). + * Parse `wheels upgrade` arguments. The `subcommand` positional selects + * the mode: `check` runs the read-only scan, `apply` performs the + * framework swap, `help` prints usage, and "" (bare) prints the usage + * steer — the destructive path always requires the explicit verb. + * `--to=` selects the target. The `saw*` fields drive the + * apply-mode refusals and the "did you mean" nudges; they match both + * `--x` and `--x=value` (LuCLI maps a bare `--x` to x=true and `--x=v` + * to x=v — either way the key exists). + * + * The MCP surface (#2963) advertises `subcommand` as a named property, + * so accept it by name as well as positionally — a tool call sending + * {subcommand: "check"} must never fall through to another verb just + * because no arg1 key exists. */ private struct function parseUpgradeArgs(required struct coll) { var parsed = upgradeArgSpec().parse(arguments.coll); + + var sub = parsed.subcommand; + if (!len(sub) && structKeyExists(arguments.coll, "subcommand") && isSimpleValue(arguments.coll.subcommand)) { + sub = arguments.coll.subcommand; + } + sub = lCase(trim(sub)); + + // --nobackup is the documented spelling, but LuCLI normalizes the + // conventional negation `--no-backup` to backup=false — honor both. + var doBackup = !parsed.nobackup; + if (structKeyExists(arguments.coll, "backup") && isSimpleValue(arguments.coll.backup) && arguments.coll.backup == "false") { + doBackup = false; + } + return { - isCheck = lCase(parsed.subcommand) == "check", + subcommand = sub, + isCheck = sub == "check", + isApply = sub == "apply", + wantsHelp = sub == "help" || sub == "-h" + || (structKeyExists(arguments.coll, "help") && isSimpleValue(arguments.coll.help) && arguments.coll.help == "true") + || (structKeyExists(arguments.coll, "h") && isSimpleValue(arguments.coll.h) && arguments.coll.h == "true"), targetVersion = parsed.to, format = parsed.format, // #2963: --strict escalates advisory findings to a hard failure @@ -2849,72 +2878,191 @@ component extends="modules.BaseModule" { // recommendations, not just breaking changes. Mirrors Django // --fail-level WARNING / Mix --warnings-as-errors. strict = parsed.strict, + doBackup = doBackup, sawTo = structKeyExists(arguments.coll, "to"), - sawDryRun = structKeyExists(arguments.coll, "dry-run") + sawDryRun = structKeyExists(arguments.coll, "dry-run"), + sawStrict = structKeyExists(arguments.coll, "strict"), + sawFormat = structKeyExists(arguments.coll, "format") }; } /** - * hint: Scan your app for breaking changes before upgrading Wheels (read-only) + * hint: Upgrade the Wheels framework in your app (vendor/wheels/) — `check` scans for breaking changes (read-only), `apply` performs the swap * - * This command does NOT perform the upgrade. It only scans the current app - * for code paths that will break against a target framework version. The - * actual framework swap is performed by your package manager - * (`brew upgrade wheels`, `scoop update wheels`, or the equivalent). + * `wheels upgrade apply` performs the framework swap (#3035): it + * replaces the app's vendor/wheels/ with the framework bundled inside + * the installed CLI, parking the old copy at vendor/wheels.bak-/ + * unless --nobackup. Recovery is a single mv (announced, with the exact + * backup path, before anything is touched). Only the CLI's bundled + * framework is available as a source for now — pair it with your package + * manager (`brew upgrade wheels`, `brew install wheels-be`, `scoop update + * wheels`) to choose what gets bundled. Downloading arbitrary --to= + * targets is the planned follow-up. * - * Despite occasional appearances in older help output, `--dry-run` is not - * supported — the command is already read-only by design. + * Bare `wheels upgrade` deliberately does nothing: it prints concise + * usage steering at the two verbs and exits 0. Destructive commands + * deserve an explicit verb, and MCP clients calling wheels_upgrade with + * {} must never mutate — requiring `apply` fixes that transport- + * independently, and exit 0 matches the pre-apply-mode bare behavior so + * existing CI invocations see usage text, not a new failure. * - * Breaking findings throw Wheels.UpgradeCheckFailed after the report is - * printed, so the command exits non-zero and can gate CI. Advisory - * (opt-in recommendation) findings never affect the exit code. + * `wheels upgrade check` keeps the read-only scan: it reports code paths + * that will break against a target framework version without modifying + * any files. Breaking findings throw Wheels.UpgradeCheckFailed after the + * report is printed, so the command exits non-zero and can gate CI. + * --strict escalates advisory findings the same way. (--dry-run is not + * supported — `check` is the preview.) * * Examples: + * wheels upgrade apply - apply the swap, with backup + * wheels upgrade apply --nobackup - apply without the backup * wheels upgrade check - scan against the latest stable release * wheels upgrade check --to=4.0.0 - scan against a specific target version * wheels upgrade check --format=json - machine-readable report (CI pipelines) */ public string function upgrade() { - var opts = parseUpgradeArgs(structuredArgs(arguments)); - - if (!opts.isCheck) { - var nl = chr(10); - var help = "Usage: wheels upgrade check [--to=] [--strict] [--format=json]" & nl - & nl - & "Scans your app for breaking changes between Wheels versions." & nl - & "This command is read-only — it does not modify vendor/wheels/." & nl - & nl - & "Options:" & nl - & " --to= Target Wheels version (default: latest stable)" & nl - & " --format=json Emit a machine-readable JSON report" & nl - & " --strict Treat advisory findings (recommended improvements) as failures" & nl - & " Useful for CI — opt-in convention changes will gate the build." & nl - & nl - & "Exit status:" & nl - & " Non-zero when breaking changes are found. With --strict, advisory findings" & nl - & " also fail the check; without --strict, advisories never affect the exit code." & nl - & nl - & "Unsupported flags:" & nl - & " --dry-run is not supported — the command is already read-only," & nl - & " so there is no dry-run mode to opt into." & nl - & nl - & "To actually install a new Wheels version, run:" & nl - & " brew upgrade wheels (macOS / Homebrew)" & nl - & " scoop update wheels (Windows / Scoop)" & nl; - - // Nudge the two common misfires from the legacy help text toward the - // right invocation explicitly (detected during parse). - if (opts.sawDryRun || opts.sawTo) { - help &= nl & "Did you mean: wheels upgrade check" - & (opts.sawTo ? " --to=" : "") - & " ?" & nl; - } - - out(help, "yellow"); - return help; - } - - return runUpgradeCheck(opts.targetVersion, opts.format, opts.strict); + var coll = structuredArgs(arguments); + var opts = parseUpgradeArgs(coll); + + if (opts.wantsHelp) { + return $printUpgradeHelp(); + } + + if (opts.isCheck) { + return runUpgradeCheck(opts.targetVersion, opts.format, opts.strict); + } + + // Bare `wheels upgrade` (no verb) is deliberately inert: print the + // usage steer and exit 0. Destructive commands deserve an explicit + // verb, and MCP clients calling wheels_upgrade with {} must never + // mutate vendor/wheels/ — requiring `apply` fixes that transport- + // independently (#3039 review). Exit 0 matches the pre-apply-mode + // bare behavior, so no CI surprise. + if (!len(opts.subcommand)) { + return $printUpgradeUsageSteer(); + } + + // ── Apply verb. Every refusal below fires before any file mutation. + + // A positional that isn't check/apply/help is a typo'd subcommand. + // A typo'd verb must hard-stop rather than exit 0 looking like it + // did something (`wheels upgrade chekc` in a script should fail + // loudly, not print usage and report success). + if (!opts.isApply) { + out("Unknown upgrade subcommand: #opts.subcommand#", "red"); + $printUpgradeHelp(); + throw( + type = "Wheels.InvalidArguments", + message = "Unknown upgrade subcommand '#opts.subcommand#' — use `wheels upgrade apply` (swap the framework) or `wheels upgrade check` (read-only scan)." + ); + } + + // Check-only flags on the apply verb almost always mean the user + // wanted the scan — nudge toward it instead of mutating + // vendor/wheels/. --dry-run is not supported on either verb; + // `check` is the preview. + if (opts.sawDryRun || opts.sawStrict || opts.sawFormat) { + var offending = opts.sawDryRun ? "--dry-run" : (opts.sawStrict ? "--strict" : "--format"); + var nudge = "wheels upgrade check" + & (opts.sawTo ? " --to=" : "") + & (opts.sawStrict ? " --strict" : "") + & (opts.sawFormat ? " --format=json" : ""); + out("#offending# is only available on the read-only scan.", "yellow"); + out("Did you mean: #nudge# ?", "yellow"); + throw( + type = "Wheels.InvalidArguments", + message = "#offending# is not supported by the apply verb — did you mean `#nudge#`?" + ); + } + + // Unknown named keys hard-stop too. ArgSpec ignores them by design + // (fine for read-only commands, kept for `check` above), but a + // destructive verb must not run alongside a flag the user typo'd. + var knownKeys = "to,format,strict,nobackup,backup,subcommand,help,h,dry-run"; + for (var key in coll) { + if (reFindNoCase("^arg\d+$", key) || listFindNoCase(knownKeys, key)) { + continue; + } + out("Unknown argument: --#key#", "red"); + throw( + type = "Wheels.InvalidArguments", + message = "Unknown argument '--#key#' — run `wheels upgrade help` for usage." + ); + } + + return runUpgradeApply(opts.targetVersion, opts.doBackup); + } + + /** + * Help block for `wheels upgrade help` / `--help`. Extracted so the + * help short-circuit and the unknown-subcommand error path stay in sync. + */ + private string function $printUpgradeHelp() { + var nl = chr(10); + var help = "Usage:" & nl + & " wheels upgrade check [--to=] [--strict] [--format=json]" & nl + & " wheels upgrade apply [--to=] [--nobackup]" & nl + & nl + & "Upgrade the Wheels framework in your app (vendor/wheels/)." & nl + & nl + & "Subcommands:" & nl + & " check Scan the app for known breaking changes between" & nl + & " your current framework version and the target." & nl + & " Read-only — does not modify any files. Exits" & nl + & " non-zero when breaking changes are found." & nl + & " apply Apply the upgrade — replace vendor/wheels/ with" & nl + & " the CLI's bundled framework. Backs up the existing" & nl + & " vendor/wheels/ as vendor/wheels.bak-/" & nl + & " unless --nobackup." & nl + & " (none) Print usage. Bare `wheels upgrade` never modifies" & nl + & " files — the swap requires the explicit `apply` verb." & nl + & nl + & "Options:" & nl + & " --to= Target Wheels version. For check, defaults to the" & nl + & " latest stable release. For apply, must match the" & nl + & " CLI's bundled framework version." & nl + & " --nobackup Apply only: skip the vendor/wheels.bak-/" & nl + & " backup. Useful when vendor/wheels/ is tracked in git." & nl + & " --strict Check only: treat advisory findings as failures" & nl + & " (non-zero exit) so CI can gate on them." & nl + & " --format=json Check only: emit a machine-readable JSON report." & nl + & nl + & "Unsupported flags:" & nl + & " --dry-run is not supported — run `wheels upgrade check` for the" & nl + & " read-only preview, then apply." & nl + & nl + & "Examples:" & nl + & " wheels upgrade check - scan against latest stable" & nl + & " wheels upgrade check --to=4.0.0 - scan against a specific version" & nl + & " wheels upgrade apply - apply the swap, with backup" & nl + & " wheels upgrade apply --nobackup - apply, skipping the backup" & nl + & nl + & "The CLI binary itself is upgraded by your package manager:" & nl + & " brew upgrade wheels (macOS / Homebrew)" & nl + & " scoop update wheels (Windows / Scoop)" & nl; + out(help, "yellow"); + return help; + } + + /** + * Concise usage steer for bare `wheels upgrade` (no subcommand). The + * bare verb is deliberately inert — see upgrade()'s dispatch comment — + * so this prints just enough to route the user to `check` or `apply` + * and exits 0 (matching the pre-apply-mode bare behavior). + */ + private string function $printUpgradeUsageSteer() { + var nl = chr(10); + var usage = "wheels upgrade needs an explicit subcommand (nothing was changed):" & nl + & nl + & " wheels upgrade check [--to=] [--strict] [--format=json]" & nl + & " Scan the app for breaking changes (read-only)." & nl + & " wheels upgrade apply [--to=] [--nobackup]" & nl + & " Replace vendor/wheels/ with the CLI's bundled framework" & nl + & " (backs up to vendor/wheels.bak-/ first)." & nl + & nl + & "Run `wheels upgrade help` for full usage." & nl; + out(usage, "yellow"); + return usage; } // ───────────────────────────────────────────────── @@ -4694,7 +4842,10 @@ component extends="modules.BaseModule" { } out(""); - out("Upgrade with: brew upgrade wheels"); + // The framework swap is `wheels upgrade apply` (#3035) — + // `brew upgrade wheels` only updates the CLI binary, never the + // app's vendored framework copy. + out("Apply with: wheels upgrade apply"); } // Throw after the full report flushes — breaking findings exit @@ -4723,6 +4874,214 @@ component extends="modules.BaseModule" { return ""; } + // ── Upgrade Apply (bundled-source swap, #3035) ─── + + /** + * Perform the framework swap: copy the CLI's bundled vendor/wheels/ + * over the project's vendor/wheels/, backing up the existing copy + * first unless the user opted out. + * + * Scope (PR1, #3035): only the CLI's bundled framework is supported + * as a source, so `--to=` is an assertion — a value that + * doesn't match the bundled version is a hard error. Downloading + * arbitrary --to= targets (via ReleaseChannel) is the PR2 follow-up. + * + * Every refusal throws Wheels.UpgradeApplyFailed AFTER printing the + * guidance, mirroring validate()'s print-then-throw convention so the + * process exits non-zero (#2941) without losing the human-readable + * explanation. + */ + private string function runUpgradeApply(string targetVersion = "", boolean doBackup = true) { + var nl = chr(10); + + // resolveProjectRoot() falls back to cwd when no vendor/wheels/ is + // found walking up, so "is this a Wheels app?" is decided by the + // vendor/wheels/ probe below — not by projectRoot being empty. + var vendorDir = variables.projectRoot & "/vendor/wheels"; + if (!$safeDirExists(vendorDir)) { + out("No vendor/wheels/ found at #vendorDir#", "red"); + out("Run 'wheels upgrade apply' from an existing app's root, or scaffold a new app with 'wheels new '."); + throw( + type = "Wheels.UpgradeApplyFailed", + message = "No vendor/wheels/ found at #vendorDir# — run `wheels upgrade apply` from a Wheels app root." + ); + } + + var sourceDir = $resolveBundledFrameworkSource(); + if (!len(sourceDir)) { + out("Could not locate the CLI's bundled framework — the CLI install may be incomplete.", "red"); + out("Tried (in order): the WHEELS_FRAMEWORK_PATH env var, then walking up from the module's own location."); + throw( + type = "Wheels.UpgradeApplyFailed", + message = "Could not locate the CLI's bundled framework (tried WHEELS_FRAMEWORK_PATH, then the module's own install tree)." + ); + } + + var upgrader = new services.FrameworkUpgrader(); + var bundledVersion = upgrader.readFrameworkVersion(sourceDir); + + if (len(arguments.targetVersion) && arguments.targetVersion != bundledVersion) { + out("Requested --to=#arguments.targetVersion# but the CLI's bundled framework is #len(bundledVersion) ? bundledVersion : 'unknown'#.", "yellow"); + out("Either:"); + out(" - Install a CLI that bundles your target (brew upgrade wheels / scoop update wheels), then re-run wheels upgrade apply."); + out(" - Or omit --to= to apply the bundled framework directly."); + throw( + type = "Wheels.UpgradeApplyFailed", + message = "--to=#arguments.targetVersion# does not match the CLI's bundled framework version (#bundledVersion#). Downloading arbitrary versions is not supported yet — see ##3035." + ); + } + + // Run the service's pre-mutation refusal checks BEFORE announcing the + // plan (#3039 review, blocking): the plan ends with the restore + // one-liner (`rm -rf "" && mv …`), and printing it on + // a refusal path would hand the user a recovery command for a backup + // that was never made — running it deletes the intact vendor/wheels/. + // applyUpgrade() re-runs the same checks (idempotent reads, no drift + // risk); refusals here print-then-throw per the #2941 convention. + var validationError = upgrader.validateSwap(sourceDir, vendorDir); + if (len(validationError)) { + out(validationError, "red"); + throw(type = "Wheels.UpgradeApplyFailed", message = validationError); + } + + out("Source: #sourceDir#"); + out("Target: #vendorDir#"); + out(""); + + // Announce the full plan — the exact backup destination and the + // recovery one-liner — BEFORE any mutation (#3039 review). If the + // copy is interrupted or dies partway, the user is already holding + // the restore command instead of fishing an unannounced backup out + // of a stack trace. The reserved path is passed into applyUpgrade() + // so the announcement and the actual backup always agree. + var plan = ""; + var backupPath = ""; + if (arguments.doBackup) { + backupPath = upgrader.reserveBackupPath(vendorDir); + plan = "Backing up vendor/wheels -> vendor/#listLast(backupPath, "/")#" & nl + & "If this is interrupted, restore with:" & nl + & " rm -rf ""#vendorDir#"" && mv ""#backupPath#"" ""#vendorDir#""" & nl; + } else { + plan = "Replacing vendor/wheels WITHOUT a backup (--nobackup) — the current copy is not recoverable if the swap fails." & nl; + } + out(plan); + + var result = {}; + try { + result = upgrader.applyUpgrade(sourceDir, vendorDir, arguments.doBackup, backupPath); + } catch (Wheels.FrameworkUpgrader e) { + // Hierarchical match: catches every service-thrown failure — + // CopyFailed (partial state; message names the backup to restore + // from, or the re-vendor instructions when --nobackup) and + // RenameFailed (backup rename refused; vendor/wheels/ intact). + // Print the message, then exit non-zero via the standard + // apply-failure type (print-then-throw, #2941). + out(e.message, "red"); + throw(type = "Wheels.UpgradeApplyFailed", message = e.message); + } + + if (!result.success) { + out(result.error, "red"); + throw(type = "Wheels.UpgradeApplyFailed", message = result.error); + } + + var summary = ""; + if (len(result.oldVersion)) { + summary &= "Framework upgraded: #result.oldVersion# -> #result.newVersion#" & nl; + } else { + summary &= "Framework installed: #result.newVersion#" & nl; + } + if (len(result.backupDir)) { + summary &= "Backup: #result.backupDir#" & nl; + summary &= "Recover with: rm -rf ""#vendorDir#"" && mv ""#result.backupDir#"" ""#vendorDir#""" & nl; + } + + // Surface root-level manifest files the user may want to review + // after the upgrade — version refs, dependencies, etc. + var rootFiles = $collectRootManifestSuggestions(); + if (arrayLen(rootFiles)) { + summary &= nl & "Files that may carry a framework version reference to review:" & nl; + for (var f in rootFiles) { + summary &= " - " & f & nl; + } + } + + out(summary, "green"); + // Return value carries the pre-swap plan too, so callers (and the + // dispatch specs) see the full command output in order. + return plan & nl & summary; + } + + /** + * Resolve the CLI's bundled framework source for apply mode. This + * deliberately bypasses the project-root candidate that + * resolveFrameworkSource() prefers — the project's vendor/wheels/ is + * what we're upgrading, so it can never be the source. + */ + private string function $resolveBundledFrameworkSource() { + // 1. WHEELS_FRAMEWORK_PATH env var (same explicit-override semantics + // as resolveFrameworkSource — an invalid path hard-fails rather + // than silently falling through, see GH #2215). + var override = ""; + try { + var javaSystem = createObject("java", "java.lang.System"); + var envValue = javaSystem.getenv("WHEELS_FRAMEWORK_PATH"); + if (!isNull(envValue)) { + override = envValue; + } + } catch (any e) { + // Env var not accessible in this runtime — treat as unset. + } + if (len(trim(override))) { + if ($safeDirExists(override)) { + return $normalizePath(override); + } + throw( + type = "Wheels.FrameworkPathInvalid", + message = "WHEELS_FRAMEWORK_PATH is set to '#override#' but that directory does not exist. Unset the variable to fall back to the CLI's bundled framework." + ); + } + + // 2. Walk up from moduleRoot looking for vendor/wheels/. On a brew + // install moduleRoot is ~/.wheels/modules/wheels/, so the very + // first candidate (./vendor/wheels) is the bundled framework; in + // a repo checkout (cli/lucli/) the walk lands on the checkout's + // own vendor/wheels/. + if (len(variables.moduleRoot)) { + var File = createObject("java", "java.io.File"); + var dir = variables.moduleRoot; + for (var i = 0; i < 6; i++) { + var canonical = $normalizePath(File.init(dir).getCanonicalPath()); + var candidate = canonical & "/vendor/wheels"; + if ($safeDirExists(candidate)) { + return candidate; + } + var parent = File.init(canonical).getParent(); + if (isNull(parent) || parent == canonical) break; + dir = parent; + } + } + + return ""; + } + + /** + * List root-level manifest / config files that may carry a wheels + * version reference the user wants to review after an upgrade. + * Filters to files that actually exist so the printed notice is + * actionable. + */ + private array function $collectRootManifestSuggestions() { + var candidates = ["box.json", "wheels.json", "config/settings.cfm"]; + var existing = []; + for (var rel in candidates) { + if (fileExists(variables.projectRoot & "/" & rel)) { + arrayAppend(existing, rel); + } + } + return existing; + } + // ── Test Execution ─────────────────────────────── private string function runTests( diff --git a/cli/lucli/services/FrameworkUpgrader.cfc b/cli/lucli/services/FrameworkUpgrader.cfc new file mode 100644 index 0000000000..60221e1e5d --- /dev/null +++ b/cli/lucli/services/FrameworkUpgrader.cfc @@ -0,0 +1,280 @@ +/** + * In-place framework swap that powers `wheels upgrade` (#3035). + * + * Replaces the contents of the app's vendor/wheels/ with a fresh copy + * of the framework (typically the CLI's bundled vendor/wheels/). The + * old vendor/wheels/ is renamed to vendor/wheels.bak- by + * default so a mistake is recoverable with a single mv. + * + * Isolated from Module.cfc so tests can exercise the file-level + * behavior without the LuCLI runtime (mirrors FrameworkInstaller.cfc). + */ +component { + + public function init() { + return this; + } + + /** + * Quick sniff test — does this look like a Wheels framework directory? + * Used to refuse swaps that would blow away an unrelated directory, on + * both sides: the source (don't vendor a random folder into the app) + * and the target (don't back up / delete something that isn't a + * framework). + * + * A bare box.json is not evidence enough (#3039 review) — every + * CommandBox-era CFML project has one. Require a wheels.json with a + * parseable non-empty `version`, or a box.json whose `version` is + * non-empty and whose `name`/`slug` (when present) identify a wheels + * artifact. A version-only box.json (no name/slug) is accepted — old + * framework drops shipped exactly that shape. + */ + public boolean function looksLikeWheelsFramework(required string dir) { + if (!directoryExists(arguments.dir)) return false; + var manifest = {}; + if (fileExists(arguments.dir & "/wheels.json")) { + manifest = $readManifest(arguments.dir & "/wheels.json"); + if (structKeyExists(manifest, "version") && isSimpleValue(manifest.version) && len(trim(manifest.version))) { + return true; + } + } + if (fileExists(arguments.dir & "/box.json")) { + manifest = $readManifest(arguments.dir & "/box.json"); + if (!structKeyExists(manifest, "version") || !isSimpleValue(manifest.version) || !len(trim(manifest.version))) { + return false; + } + var identifiers = []; + if (structKeyExists(manifest, "name") && isSimpleValue(manifest.name)) { + arrayAppend(identifiers, manifest.name); + } + if (structKeyExists(manifest, "slug") && isSimpleValue(manifest.slug)) { + arrayAppend(identifiers, manifest.slug); + } + if (arrayLen(identifiers) == 0) { + return true; + } + for (var identifier in identifiers) { + if (findNoCase("wheels", identifier)) { + return true; + } + } + } + return false; + } + + /** + * Read and parse a JSON manifest. Returns an empty struct when the file + * is unreadable, malformed, or not a JSON object. + */ + private struct function $readManifest(required string path) { + try { + var data = deserializeJSON(fileRead(arguments.path)); + if (isStruct(data)) { + return data; + } + } catch (any e) { + // Malformed JSON — callers treat an empty struct as "no evidence". + } + return {}; + } + + /** + * Read the `version` field from a framework dir's wheels.json (preferred) + * or box.json (legacy fallback). Returns "" if no manifest is present or + * the version field is missing/unparsable. + */ + public string function readFrameworkVersion(required string dir) { + var manifestPath = arguments.dir & "/wheels.json"; + if (!fileExists(manifestPath)) { + manifestPath = arguments.dir & "/box.json"; + if (!fileExists(manifestPath)) return ""; + } + var data = $readManifest(manifestPath); + if (structKeyExists(data, "version") && isSimpleValue(data.version)) { + return data.version; + } + return ""; + } + + /** + * Run every pre-mutation refusal check for a sourceDir -> vendorDir + * swap. Returns "" when the swap may proceed, or the human-readable + * refusal otherwise. Pure reads — no file is created, renamed, or + * deleted — so callers can (and must) run it BEFORE announcing the + * swap plan: printing the backup destination and the `rm -rf … && mv …` + * restore one-liner ahead of a refusal would hand the user a recovery + * command for a backup that was never made (#3039 review). + * applyUpgrade() re-runs it first, so direct service callers keep the + * exact same refusal behavior. + */ + public string function validateSwap(required string sourceDir, required string vendorDir) { + // 1. Validate the source. + if (!directoryExists(arguments.sourceDir)) { + return "Source framework directory does not exist: " & arguments.sourceDir; + } + if (!looksLikeWheelsFramework(arguments.sourceDir)) { + return "Source does not look like a Wheels framework directory (need a wheels.json or box.json whose version and name identify a Wheels framework): " & arguments.sourceDir; + } + + // 2. Validate the target's parent. We never create the vendor/ parent + // ourselves — if vendor/ doesn't exist, the user is almost certainly + // pointed at the wrong directory. + var File = createObject("java", "java.io.File"); + var parentPath = File.init(arguments.vendorDir).getParent(); + if (isNull(parentPath) || !directoryExists(parentPath)) { + return "Parent of target directory does not exist: " & arguments.vendorDir; + } + + // 3. Identity / containment guard — BEFORE any destructive step. + // Running `wheels upgrade` inside the wheels repo checkout itself + // resolves the bundled source to the very vendor/wheels/ being + // replaced; the backup rename (or --nobackup delete) would destroy + // the source mid-swap. Containment in either direction is just as + // fatal: copying a parent into its own child recurses, and copying + // a child of the target reads from a directory we just renamed. + var srcCanonical = File.init(arguments.sourceDir).getCanonicalPath(); + var dstCanonical = File.init(arguments.vendorDir).getCanonicalPath(); + if (srcCanonical == dstCanonical) { + return "Source and target are the same directory (" & dstCanonical & ") — refusing to swap a framework with itself. Are you running `wheels upgrade` inside the wheels repo checkout?"; + } + var separator = "/"; + if (find("\", srcCanonical & dstCanonical)) { + separator = "\"; + } + if (left(srcCanonical & separator, len(dstCanonical & separator)) == dstCanonical & separator + || left(dstCanonical & separator, len(srcCanonical & separator)) == srcCanonical & separator) { + return "Source and target directories contain one another (source: " & srcCanonical & ", target: " & dstCanonical & ") — refusing to swap."; + } + + // 4. If vendorDir already exists, sniff it before destroying anything. + if (directoryExists(arguments.vendorDir) && !looksLikeWheelsFramework(arguments.vendorDir)) { + return "Target directory exists but does not look like a Wheels framework (need a wheels.json or box.json whose version and name identify a Wheels framework): " & arguments.vendorDir; + } + + return ""; + } + + /** + * Replace `vendorDir` with the contents of `sourceDir`. Both must be + * Wheels framework directories (or vendorDir may be absent — fresh + * install). When `doBackup` is true the existing vendorDir is renamed + * to `.bak-` before the copy. Callers that want + * to announce the backup destination BEFORE invoking the swap can + * reserve it via reserveBackupPath() and pass it in as `backupPath` — + * the announced path and the actual backup are then guaranteed to + * agree. When `backupPath` is empty the path is reserved here. + * + * Returns a struct describing the outcome: + * - success : boolean — did the swap complete? + * - backupDir : string — path of the backup directory, or "" if none + * - error : string — human-readable error on failure, "" on success + * - oldVersion : string — version read from vendorDir BEFORE the swap + * - newVersion : string — version read from vendorDir AFTER the swap + * + * Throws Wheels.FrameworkUpgrader.CopyFailed when the copy step fails + * AFTER the destructive backup-rename/delete already ran — the message + * names the partial-state target and the backup to restore from (or, + * with no backup, says the old tree is gone and how to re-vendor). + * Validation refusals before any mutation come back as result.error. + */ + public struct function applyUpgrade( + required string sourceDir, + required string vendorDir, + boolean doBackup = true, + string backupPath = "" + ) { + var result = { + success: false, + backupDir: "", + error: "", + oldVersion: "", + newVersion: "" + }; + + // Pre-mutation refusal checks. Callers that announce the swap plan + // (backup destination + restore one-liner) run validateSwap() first + // so refusals never print recovery guidance for a backup that was + // never made (#3039 review) — re-running it here keeps the service + // safe for direct callers, and the checks are idempotent reads. + result.error = validateSwap(arguments.sourceDir, arguments.vendorDir); + if (len(result.error)) { + return result; + } + + // If vendorDir already exists (validateSwap confirmed it sniffs as a + // framework), record its version and park or delete it. + if (directoryExists(arguments.vendorDir)) { + result.oldVersion = readFrameworkVersion(arguments.vendorDir); + + if (arguments.doBackup) { + result.backupDir = len(trim(arguments.backupPath)) ? arguments.backupPath : reserveBackupPath(arguments.vendorDir); + $renameDirectory(arguments.vendorDir, result.backupDir); + } else { + directoryDelete(arguments.vendorDir, true); + } + } + + // Copy the source into the target. directoryCopy with recurse=true + // mirrors source's contents into vendorDir. From here on the old + // tree is already renamed away (or deleted) — a failure must not + // surface as a bare stack trace over an unannounced partial state + // (#3039 review), so name the recovery path explicitly. + try { + directoryCreate(arguments.vendorDir, true); + directoryCopy(arguments.sourceDir, arguments.vendorDir, true); + } catch (any e) { + if (len(result.backupDir)) { + throw( + type = "Wheels.FrameworkUpgrader.CopyFailed", + message = "Copying the new framework failed partway (#e.message#). ""#arguments.vendorDir#"" is in a partial state — restore the backup with: rm -rf ""#arguments.vendorDir#"" && mv ""#result.backupDir#"" ""#arguments.vendorDir#""" + ); + } + throw( + type = "Wheels.FrameworkUpgrader.CopyFailed", + message = "Copying the new framework failed partway (#e.message#) and no backup exists (--nobackup) — the old ""#arguments.vendorDir#"" tree is gone. Fix the cause above, then re-run `wheels upgrade apply` to re-vendor the framework from the CLI bundle." + ); + } + + result.newVersion = readFrameworkVersion(arguments.vendorDir); + result.success = true; + return result; + } + + /** + * Build a unique backup path of the form .bak--, + * appending a counter if a collision exists (concurrent or rapid re-runs). + * Public so callers can announce the exact backup destination (and the + * recovery one-liner) before invoking applyUpgrade — pass the reserved + * path back in via `backupPath` so the plan and the swap can't disagree. + */ + public string function reserveBackupPath(required string vendorDir) { + var ts = dateFormat(now(), "yyyymmdd") & "-" & timeFormat(now(), "HHmmss"); + var candidate = arguments.vendorDir & ".bak-" & ts; + var counter = 1; + while (directoryExists(candidate)) { + candidate = arguments.vendorDir & ".bak-" & ts & "-" & counter; + counter++; + } + return candidate; + } + + /** + * Atomic-ish directory rename via Java's File.renameTo. CFML's + * directoryRename isn't available on every engine the CLI may host + * under, and the cross-filesystem semantics are inconsistent — falling + * through to copy+delete here would defeat the "single mv to recover" + * promise we want for backups, so we surface the rename failure. + */ + private void function $renameDirectory(required string fromPath, required string toPath) { + var File = createObject("java", "java.io.File"); + var src = File.init(arguments.fromPath); + var dst = File.init(arguments.toPath); + if (!src.renameTo(dst)) { + throw( + type = "Wheels.FrameworkUpgrader.RenameFailed", + message = "Failed to rename " & arguments.fromPath & " to " & arguments.toPath & ". The backup directory must live on the same filesystem as vendor/wheels/." + ); + } + } + +} diff --git a/cli/lucli/tests/_fixtures/commands/ModuleOutputCapture.cfc b/cli/lucli/tests/_fixtures/commands/ModuleOutputCapture.cfc new file mode 100644 index 0000000000..d518c11551 --- /dev/null +++ b/cli/lucli/tests/_fixtures/commands/ModuleOutputCapture.cfc @@ -0,0 +1,35 @@ +/** + * Test fixture for asserting what Module.cfc actually PRINTS, not just + * what it returns or throws. + * + * The BaseModule test double's out() is a no-op, so specs normally can't + * see console output — fine for return-value contracts, blind for output + * ordering bugs. The #3039 review's blocking finding is exactly such a + * bug: the pre-swap restore one-liner (`rm -rf … && mv …`) printed before + * the service-level refusal checks ran, so every refusal path handed the + * user a restore command for a backup that was never made. + * + * This fixture extends Module and overrides out() to accumulate every + * line, so refusal specs can assert the restore command is ABSENT from + * the printed output (and success specs can assert it is present). + */ +component extends="cli.lucli.Module" { + + void function out(any message, string colour = "", string style = "") { + if (!structKeyExists(variables, "capturedLines")) { + variables.capturedLines = []; + } + arrayAppend(variables.capturedLines, toString(arguments.message)); + } + + /** + * Everything out() printed so far, newline-joined, in order. + */ + public string function capturedOutput() { + if (!structKeyExists(variables, "capturedLines")) { + return ""; + } + return arrayToList(variables.capturedLines, chr(10)); + } + +} diff --git a/cli/lucli/tests/specs/commands/InfoCommandSpec.cfc b/cli/lucli/tests/specs/commands/InfoCommandSpec.cfc index 7af33f4eae..4ceeceacd8 100644 --- a/cli/lucli/tests/specs/commands/InfoCommandSpec.cfc +++ b/cli/lucli/tests/specs/commands/InfoCommandSpec.cfc @@ -136,22 +136,31 @@ component extends="wheels.wheelstest.system.BaseSpec" { describe("wheels upgrade", () => { - it("shows help when called with no args", () => { - mod.__arguments = []; - mod.upgrade(); - expect(true).toBeTrue(); + // The swap requires the explicit `apply` verb (#3039 review): + // bare `wheels upgrade` prints usage steering and exits 0, so + // an MCP wheels_upgrade call with {} can never mutate. Deeper + // dispatch coverage lives in UpgradeApplyCommandSpec. + + it("shows help when called with the help subcommand", () => { + var result = mod.upgrade(arg1 = "help"); + expect(result).toInclude("wheels upgrade"); }); - it("accepts check subcommand", () => { - mod.__arguments = ["check"]; - mod.upgrade(); + it("accepts check subcommand with --to flag", () => { + mod.upgrade(arg1 = "check", to = "3.0.0"); expect(true).toBeTrue(); }); - it("accepts --to flag", () => { - mod.__arguments = ["check", "--to=3.0.0"]; - mod.upgrade(); - expect(true).toBeTrue(); + it("bare verb prints usage steering and never mutates", () => { + var result = mod.upgrade(); + expect(result).toInclude("wheels upgrade check"); + expect(result).toInclude("wheels upgrade apply"); + }); + + it("apply verb refuses over the empty vendor/wheels stub", () => { + // The stub has no wheels.json/box.json to sniff, so apply + // must refuse before touching it. + expect(() => mod.upgrade(arg1 = "apply")).toThrow(type = "Wheels.UpgradeApplyFailed"); }); }); diff --git a/cli/lucli/tests/specs/commands/UpgradeApplyCommandSpec.cfc b/cli/lucli/tests/specs/commands/UpgradeApplyCommandSpec.cfc new file mode 100644 index 0000000000..7a882a00d8 --- /dev/null +++ b/cli/lucli/tests/specs/commands/UpgradeApplyCommandSpec.cfc @@ -0,0 +1,304 @@ +/** + * Behavioral specs for `wheels upgrade` dispatch — explicit verb + * selection, help paths, and the refusals that fire before any file + * mutation. Covers issue #3035 (PR1 of the apply-mode plan) plus the + * #3039 review hardening: the swap requires the explicit `apply` verb. + * Bare `wheels upgrade` prints concise usage steering at the two verbs + * and exits 0 — destructive commands deserve an explicit verb, and MCP + * clients calling wheels_upgrade with {} must never mutate. + * + * The actual file swap is exercised by FrameworkUpgraderSpec — Module.cfc + * here is the thin dispatch layer that decides which service call to make. + * Instantiates Module.cfc against a scaffolded temp project and drives it + * through the structured callerArgs path (`mod.upgrade(arg1 = "check")`), + * the same shape LuCLI's own dispatch produces — the `__arguments` stash + * is only readable for internal delegation, not from a spec (see + * DbCommandSpec's unknown-subcommand spec for the prior art). + * + * The companion UpgradeCommandSpec covers the check-mode scanner structure + * at the source level; this spec covers runtime dispatch behavior. + */ +component extends="wheels.wheelstest.system.BaseSpec" { + + function beforeAll() { + variables.testHelper = new cli.lucli.tests.TestHelper(); + // Apply mode resolves its source by walking up from cli/lucli/ — + // in this checkout that lands on the repo's own vendor/wheels/. + // Read whatever version this checkout actually bundles so the + // assertions don't pin a release number. + variables.bundledVersion = new cli.lucli.services.FrameworkUpgrader() + .readFrameworkVersion(expandPath("/vendor/wheels")); + } + + private void function seedVendorWheels(string version = "4.0.0-SNAPSHOT+1687") { + var vendorDir = variables.tempRoot & "/vendor/wheels"; + directoryCreate(vendorDir, true, true); + fileWrite(vendorDir & "/wheels.json", '{"name":"wheels","version":"' & arguments.version & '"}'); + } + + private string function seededVersion() { + var manifest = deserializeJSON(fileRead(variables.tempRoot & "/vendor/wheels/wheels.json")); + return manifest.version; + } + + private array function listBackups() { + return directoryList(variables.tempRoot & "/vendor", false, "name", "wheels.bak-*"); + } + + function run() { + + describe("wheels upgrade dispatch", () => { + + // DSL form — component-level beforeEach()/afterEach() are not + // BDD lifecycle hooks in this harness. Fresh project per spec: + // apply mode mutates vendor/, so specs can't share a fixture. + beforeEach(() => { + variables.tempRoot = testHelper.scaffoldTempProject(expandPath("/")); + // Output-capturing Module (same dispatch surface) so refusal + // specs can assert what got PRINTED, not just what was thrown: + // the #3039 review's blocking finding was a restore one-liner + // printed on paths where no backup was ever made. + variables.mod = new cli.lucli.tests._fixtures.commands.ModuleOutputCapture(cwd = variables.tempRoot); + }); + + afterEach(() => { + testHelper.cleanupTempProject(variables.tempRoot); + }); + + describe("wheels upgrade help", () => { + + it("returns help text when invoked with --help", () => { + var result = mod.upgrade(help = true); + expect(result).toInclude("wheels upgrade"); + expect(result).toInclude("--nobackup"); + expect(result).toInclude("check"); + }); + + it("returns help text when invoked with -h", () => { + var result = mod.upgrade(arg1 = "-h"); + expect(result).toInclude("wheels upgrade"); + }); + + it("returns help text when invoked with bare `help`", () => { + var result = mod.upgrade(arg1 = "help"); + expect(result).toInclude("wheels upgrade"); + }); + + it("documents the explicit verbs: apply swaps, check scans, bare prints usage", () => { + var result = mod.upgrade(arg1 = "help"); + expect(result).toInclude("wheels upgrade apply"); + expect(result).toInclude("wheels upgrade check"); + expect(result).toInclude("Apply the upgrade"); + expect(result).toInclude("--to="); + expect(result).toInclude("--nobackup"); + }); + }); + + describe("wheels upgrade (bare verb) — usage steer, never the swap", () => { + + // #3039 review: the bare verb is deliberately inert. Exit 0 + // matches the pre-apply-mode behavior (no CI surprise), and + // an MCP wheels_upgrade call with {} must never mutate. + + it("prints usage steering at check/apply without mutating vendor/wheels/", () => { + seedVendorWheels(); + var result = mod.upgrade(); + expect(result).toInclude("wheels upgrade check"); + expect(result).toInclude("wheels upgrade apply"); + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + expect(arrayLen(listBackups())).toBe(0); + }); + + it("steers to usage even when apply flags are present without the verb", () => { + seedVendorWheels(); + var result = mod.upgrade(nobackup = true); + expect(result).toInclude("wheels upgrade apply"); + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + expect(arrayLen(listBackups())).toBe(0); + }); + }); + + describe("wheels upgrade argument refusals (before any mutation)", () => { + + it("rejects --dry-run on the apply verb with a pointer at `wheels upgrade check`", () => { + seedVendorWheels(); + expect(() => mod.upgrade(argumentCollection = {"arg1": "apply", "dry-run": "true"})).toThrow(type = "Wheels.InvalidArguments"); + // vendor/wheels/ untouched. + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + }); + + it("rejects check-only flags on the apply verb (--strict)", () => { + seedVendorWheels(); + expect(() => mod.upgrade(arg1 = "apply", strict = true)).toThrow(regex = "wheels upgrade check"); + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + }); + + it("rejects check-only flags on the apply verb (--format=json)", () => { + seedVendorWheels(); + expect(() => mod.upgrade(arg1 = "apply", format = "json")).toThrow(type = "Wheels.InvalidArguments"); + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + }); + + it("rejects an unknown flag instead of silently applying", () => { + seedVendorWheels(); + expect(() => mod.upgrade(arg1 = "apply", bogus = true)).toThrow(regex = "bogus"); + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + }); + + it("rejects an unknown subcommand instead of treating it as apply", () => { + seedVendorWheels(); + expect(() => mod.upgrade(arg1 = "chekc")).toThrow(regex = "chekc"); + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + }); + }); + + describe("wheels upgrade apply refusals", () => { + + it("refuses when no vendor/wheels/ exists in the project", () => { + // scaffoldTempProject does not create vendor/wheels/. + expect(() => mod.upgrade(arg1 = "apply")).toThrow(type = "Wheels.UpgradeApplyFailed"); + // And nothing was created. + expect(directoryExists(variables.tempRoot & "/vendor/wheels")).toBeFalse(); + // No backup happened, so no restore command may be offered. + expect(mod.capturedOutput()).notToInclude("rm -rf"); + }); + + it("refuses when --to= does not match the CLI's bundled framework version", () => { + seedVendorWheels(version = "4.0.0-SNAPSHOT+1687"); + expect(() => mod.upgrade(arg1 = "apply", to = "99.99.99")).toThrow(type = "Wheels.UpgradeApplyFailed"); + // Side effect check: the seeded manifest is untouched. + expect(seededVersion()).toBe("4.0.0-SNAPSHOT+1687"); + expect(mod.capturedOutput()).notToInclude("rm -rf"); + }); + + it("refuses a vendor/wheels/ that does not sniff as a framework — without printing the restore one-liner", () => { + // #3039 review (blocking): the service-level refusals fire + // AFTER Module printed the pre-swap plan, so the user was + // handed `rm -rf "" && mv "" …` for + // a backup that was never made — running it deletes the + // intact vendor/wheels/. Drive a real service refusal (a + // generic app box.json is not framework evidence) and pin + // that the restore command never reaches the output. + var vendorDir = variables.tempRoot & "/vendor/wheels"; + directoryCreate(vendorDir, true, true); + fileWrite(vendorDir & "/box.json", '{"name":"myapp","version":"1.0.0"}'); + fileWrite(vendorDir & "/marker.txt", "not-a-framework"); + + expect(() => mod.upgrade(arg1 = "apply")).toThrow(type = "Wheels.UpgradeApplyFailed"); + + // The refusal explains itself… + expect(mod.capturedOutput()).toInclude("does not look like a Wheels framework"); + // …but never offers a restore command for a backup that + // does not exist. + expect(mod.capturedOutput()).notToInclude("rm -rf"); + expect(mod.capturedOutput()).notToInclude("Backing up vendor/wheels"); + + // And the target is untouched: no backup, no mutation. + expect(arrayLen(listBackups())).toBe(0); + expect(fileRead(vendorDir & "/marker.txt")).toBe("not-a-framework"); + }); + }); + + describe("wheels upgrade apply — the swap", () => { + + it("swaps vendor/wheels/ with the bundled framework and backs the old copy up", () => { + seedVendorWheels(version = "0.0.1-spec-fixture"); + fileWrite(variables.tempRoot & "/vendor/wheels/marker.txt", "old-framework"); + var result = mod.upgrade(arg1 = "apply"); + + // Live copy now carries the bundled framework. + expect(seededVersion()).toBe(variables.bundledVersion); + expect(fileExists(variables.tempRoot & "/vendor/wheels/marker.txt")).toBeFalse(); + + // Old copy parked under vendor/wheels.bak-. + var backups = listBackups(); + expect(arrayLen(backups)).toBe(1); + expect(reFindNoCase("^wheels\.bak-\d{8}-\d{6}", backups[1])).toBeGT(0); + expect(fileRead(variables.tempRoot & "/vendor/" & backups[1] & "/marker.txt")).toBe("old-framework"); + + // And the summary reports old -> new plus the recovery path. + expect(result).toInclude("0.0.1-spec-fixture"); + expect(result).toInclude("Backup"); + }); + + it("announces the exact backup destination and recovery command before the swap summary", () => { + // #3039 review: the plan — backup destination + quoted + // recovery one-liner — must be part of the output BEFORE + // the swap runs, so an interrupt leaves the user holding + // the restore command. + seedVendorWheels(version = "0.0.1-spec-fixture"); + var result = mod.upgrade(arg1 = "apply"); + + var backups = listBackups(); + expect(arrayLen(backups)).toBe(1); + // The announced destination is the directory the backup + // actually landed in (reserved up front, passed through). + expect(result).toInclude("Backing up vendor/wheels -> vendor/" & backups[1]); + expect(result).toInclude("If this is interrupted, restore with:"); + expect(result).toInclude('rm -rf "'); + expect(result).toInclude('/vendor/wheels" && mv "'); + // And it precedes the post-swap summary in the output. + expect(find("Backing up vendor/wheels", result)).toBeGT(0); + expect(find("Framework upgraded:", result)).toBeGT(find("Backing up vendor/wheels", result)); + // The restore one-liner also reached the PRINTED output + // (the refusal specs pin its absence; this pins presence + // on the one path where the backup really is made). + expect(mod.capturedOutput()).toInclude('rm -rf "'); + }); + + it("accepts --to= matching the bundled version and skips the backup with --nobackup", () => { + seedVendorWheels(version = "0.0.1-spec-fixture"); + mod.upgrade(arg1 = "apply", to = variables.bundledVersion, nobackup = true); + + expect(seededVersion()).toBe(variables.bundledVersion); + expect(arrayLen(listBackups())).toBe(0); + }); + + it("skips the backup when LuCLI normalizes --no-backup to backup=""false""", () => { + // LuCLI converts the conventional `--no-backup` negation into + // the named-arg shape `backup = "false"` before dispatch. + // parseUpgradeArgs() must honour this alongside the documented + // --nobackup spelling. + seedVendorWheels(version = "0.0.1-spec-fixture"); + mod.upgrade(argumentCollection = {"arg1": "apply", "backup": "false"}); + + expect(seededVersion()).toBe(variables.bundledVersion); + expect(arrayLen(listBackups())).toBe(0); + }); + + it("dispatches apply when the MCP surface sends `subcommand` as a named key", () => { + // MCP clients call wheels_upgrade with named properties + // from the advertised inputSchema (#2963) — the explicit + // {subcommand: "apply"} opt-in is the only MCP shape that + // may mutate. + seedVendorWheels(version = "0.0.1-spec-fixture"); + mod.upgrade(subcommand = "apply"); + expect(seededVersion()).toBe(variables.bundledVersion); + }); + }); + + describe("wheels upgrade check (read-only scan)", () => { + + it("treats `check` as a read-only mode (no vendor/wheels/ mutation)", () => { + seedVendorWheels(version = "4.0.0"); + // Add a marker file we can check is preserved after the scan. + fileWrite(variables.tempRoot & "/vendor/wheels/marker.txt", "untouched"); + mod.upgrade(arg1 = "check", to = "4.0.1"); + expect(fileExists(variables.tempRoot & "/vendor/wheels/marker.txt")).toBeTrue(); + expect(fileRead(variables.tempRoot & "/vendor/wheels/marker.txt")).toBe("untouched"); + }); + + it("dispatches check when the MCP surface sends `subcommand` as a named key", () => { + // MCP clients call wheels_upgrade with named properties from + // the advertised inputSchema (#2963) — {subcommand: "check"} + // must select the scan, never fall through to apply. + seedVendorWheels(version = "4.0.0"); + fileWrite(variables.tempRoot & "/vendor/wheels/marker.txt", "untouched"); + mod.upgrade(subcommand = "check", to = "4.0.1"); + expect(fileRead(variables.tempRoot & "/vendor/wheels/marker.txt")).toBe("untouched"); + }); + }); + + }); + } +} diff --git a/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc b/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc index d31250d1b0..fb9840b3d6 100644 --- a/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc +++ b/cli/lucli/tests/specs/commands/UpgradeCommandSpec.cfc @@ -114,7 +114,7 @@ component extends="wheels.wheelstest.system.BaseSpec" { }); it("documents --strict in the upgrade() help banner", () => { - // The help text users read when running bare `wheels upgrade` + // The help text users read when running `wheels upgrade help` // must surface the new flag — otherwise it's discoverable only // by reading the source. expect(variables.moduleSource).toInclude("--strict"); diff --git a/cli/lucli/tests/specs/services/FrameworkUpgraderSpec.cfc b/cli/lucli/tests/specs/services/FrameworkUpgraderSpec.cfc new file mode 100644 index 0000000000..05bb4b72e5 --- /dev/null +++ b/cli/lucli/tests/specs/services/FrameworkUpgraderSpec.cfc @@ -0,0 +1,478 @@ +/** + * Specs for FrameworkUpgrader — the in-place vendor/wheels/ swap that + * powers `wheels upgrade apply`. Exercises file-level behavior in + * isolation so the Module-level spec doesn't need to fake out CLI-bundle + * paths. + * + * Covers issue #3035 (PR1 of the apply-mode plan): the previous + * `wheels upgrade check` suggested `brew upgrade wheels`, which only + * touches the CLI binary and never the app's vendor/wheels/ copy. + * + * Fixture lifecycle: every temp directory is registered in + * variables.tempPaths (via newTempDir()/buildFixture()) and removed by the + * afterEach hook in each describe, so a failing expectation can't leak + * fixtures into the OS temp dir. + */ +component extends="wheels.wheelstest.system.BaseSpec" { + + function beforeAll() { + variables.upgrader = new cli.lucli.services.FrameworkUpgrader(); + variables.tempPaths = []; + } + + /** + * Create (and register for afterEach cleanup) a unique temp directory. + */ + private string function newTempDir(required string prefix) { + var dir = getTempDirectory() & arguments.prefix & "-#createUUID()#"; + directoryCreate(dir, true, true); + arrayAppend(variables.tempPaths, dir); + return dir; + } + + private struct function buildFixture( + string sourceManifest = '{"name":"wheels","version":"4.0.1"}', + string targetManifest = '{"name":"wheels","version":"4.0.0-SNAPSHOT+1687"}', + boolean writeTargetManifest = true, + string sourceMarker = "new-framework", + string targetMarker = "old-framework" + ) { + var f = {}; + f.root = newTempDir("wheels-upgrader-fixture"); + f.sourceDir = f.root & "/bundled/vendor/wheels"; + f.vendorParent = f.root & "/app/vendor"; + f.vendorDir = f.vendorParent & "/wheels"; + directoryCreate(f.sourceDir, true, true); + directoryCreate(f.vendorParent, true, true); + directoryCreate(f.vendorDir, true, true); + fileWrite(f.sourceDir & "/wheels.json", arguments.sourceManifest); + fileWrite(f.sourceDir & "/marker.txt", arguments.sourceMarker); + // Throw a nested file in so we know the recursive copy actually runs. + directoryCreate(f.sourceDir & "/model", true, true); + fileWrite(f.sourceDir & "/model/Base.cfc", "// new model"); + if (arguments.writeTargetManifest) { + fileWrite(f.vendorDir & "/wheels.json", arguments.targetManifest); + } + fileWrite(f.vendorDir & "/marker.txt", arguments.targetMarker); + return f; + } + + private void function $cleanupTempDirs() { + for (var p in variables.tempPaths) { + if (directoryExists(p)) { + directoryDelete(p, true); + } + } + variables.tempPaths = []; + } + + /** + * Drop read permission on a file so directoryCopy fails partway — + * the hermetic mid-swap failure the CopyFailed contract is for. + * Returns false when the platform can't revoke read access (Windows), + * in which case the caller skips: the contract is covered on POSIX. + */ + private boolean function $makeUnreadable(required string path) { + var File = createObject("java", "java.io.File"); + var handle = File.init(arguments.path); + return handle.setReadable(false, false) && !handle.canRead(); + } + + private void function $makeReadable(required string path) { + createObject("java", "java.io.File").init(arguments.path).setReadable(true, false); + } + + function run() { + + describe("FrameworkUpgrader.looksLikeWheelsFramework", () => { + + afterEach(() => $cleanupTempDirs()); + + // #3039 review hardening: a bare manifest file is not evidence — + // any CFML project has a box.json. Require version (and, for + // box.json, a wheels-ish name/slug when one is present). + + it("returns true for wheels.json with a non-empty version", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/wheels.json", '{"version":"4.0.1"}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeTrue(); + }); + + it("returns false for wheels.json without a version", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/wheels.json", "{}"); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeFalse(); + }); + + it("returns false for malformed wheels.json", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/wheels.json", "{not json"); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeFalse(); + }); + + it("returns true for legacy box.json with a version and a wheels name", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/box.json", '{"name":"cfwheels","version":"3.9.0"}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeTrue(); + }); + + it("returns true for legacy box.json with a version and a wheels slug", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/box.json", '{"slug":"wheels-be","version":"4.1.0-SNAPSHOT"}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeTrue(); + }); + + it("returns true for a version-only box.json (no name/slug — old framework drops)", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/box.json", '{"version":"3.9.0"}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeTrue(); + }); + + it("returns false for a generic app box.json (name myapp)", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/box.json", '{"name":"myapp","version":"1.0.0"}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeFalse(); + }); + + it("returns false for box.json with an empty version", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/box.json", '{"name":"wheels","version":""}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeFalse(); + }); + + it("falls back to a qualifying box.json when wheels.json lacks a version", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/wheels.json", "{}"); + fileWrite(dir & "/box.json", '{"name":"wheels","version":"3.9.0"}'); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeTrue(); + }); + + it("returns false for a directory with neither manifest", () => { + var dir = newTempDir("lwf"); + fileWrite(dir & "/README.md", ""); + expect(upgrader.looksLikeWheelsFramework(dir)).toBeFalse(); + }); + + it("returns false for a non-existent directory", () => { + expect(upgrader.looksLikeWheelsFramework(getTempDirectory() & "does-not-exist-#createUUID()#")).toBeFalse(); + }); + }); + + describe("FrameworkUpgrader.readFrameworkVersion", () => { + + afterEach(() => $cleanupTempDirs()); + + it("reads version from wheels.json when present", () => { + var dir = newTempDir("rfv"); + fileWrite(dir & "/wheels.json", '{"version":"4.0.1"}'); + expect(upgrader.readFrameworkVersion(dir)).toBe("4.0.1"); + }); + + it("falls back to box.json when wheels.json is absent", () => { + var dir = newTempDir("rfv"); + fileWrite(dir & "/box.json", '{"version":"3.9.0"}'); + expect(upgrader.readFrameworkVersion(dir)).toBe("3.9.0"); + }); + + it("prefers wheels.json over box.json when both exist", () => { + var dir = newTempDir("rfv"); + fileWrite(dir & "/wheels.json", '{"version":"4.0.1"}'); + fileWrite(dir & "/box.json", '{"version":"3.9.0"}'); + expect(upgrader.readFrameworkVersion(dir)).toBe("4.0.1"); + }); + + it("returns empty string when no manifest is present", () => { + var dir = newTempDir("rfv"); + expect(upgrader.readFrameworkVersion(dir)).toBe(""); + }); + + it("returns empty string for malformed manifest JSON", () => { + var dir = newTempDir("rfv"); + fileWrite(dir & "/wheels.json", "{not json"); + expect(upgrader.readFrameworkVersion(dir)).toBe(""); + }); + }); + + describe("FrameworkUpgrader.reserveBackupPath", () => { + + afterEach(() => $cleanupTempDirs()); + + it("returns .bak- and never an existing path", () => { + var dir = newTempDir("rbp"); + var vendorDir = dir & "/wheels"; + directoryCreate(vendorDir, true, true); + var first = upgrader.reserveBackupPath(vendorDir); + expect(reFindNoCase("/wheels\.bak-\d{8}-\d{6}", first)).toBeGT(0); + // Occupy the first reservation — the next one must dodge it. + directoryCreate(first, true, true); + var second = upgrader.reserveBackupPath(vendorDir); + expect(second).notToBe(first); + expect(directoryExists(second)).toBeFalse(); + }); + }); + + describe("FrameworkUpgrader.validateSwap", () => { + + afterEach(() => $cleanupTempDirs()); + + // #3039 review (blocking): the pre-mutation refusal checks must be + // callable on their own, BEFORE the caller prints the pre-swap plan + // (backup destination + `rm -rf … && mv …` restore one-liner) — + // otherwise every refusal path hands the user a restore command + // for a backup that was never made. validateSwap() is that same + // check block, extracted; applyUpgrade() still runs it first, so + // the checks are idempotent reads with no drift risk. + + it("returns an empty string for a valid source/target pair", () => { + var f = buildFixture(); + expect(upgrader.validateSwap(f.sourceDir, f.vendorDir)).toBe(""); + }); + + it("returns an empty string for a fresh install (target absent, parent present)", () => { + var f = buildFixture(); + directoryDelete(f.vendorDir, true); + expect(upgrader.validateSwap(f.sourceDir, f.vendorDir)).toBe(""); + }); + + it("returns the sniff refusal for a non-framework source", () => { + var f = buildFixture(); + fileDelete(f.sourceDir & "/wheels.json"); + expect(upgrader.validateSwap(f.sourceDir, f.vendorDir)).toInclude("does not look like a Wheels framework"); + }); + + it("returns the identity refusal when source and target are the same directory", () => { + var f = buildFixture(); + expect(upgrader.validateSwap(f.vendorDir, f.vendorDir)).toInclude("same directory"); + }); + + it("returns the target-sniff refusal when the target exists but is not a framework", () => { + var f = buildFixture(writeTargetManifest = false); + expect(upgrader.validateSwap(f.sourceDir, f.vendorDir)).toInclude("does not look like a Wheels framework"); + }); + + it("returns the missing-parent refusal without creating anything", () => { + var f = buildFixture(); + var orphanTarget = f.root & "/no-such-parent/wheels"; + expect(upgrader.validateSwap(f.sourceDir, orphanTarget)).toInclude("Parent of target directory does not exist"); + expect(directoryExists(f.root & "/no-such-parent")).toBeFalse(); + }); + }); + + describe("FrameworkUpgrader.applyUpgrade", () => { + + afterEach(() => $cleanupTempDirs()); + + it("replaces vendor/wheels/ contents with the source contents", () => { + var f = buildFixture(); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + expect(result.success).toBeTrue(); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("new-framework"); + expect(fileExists(f.vendorDir & "/model/Base.cfc")).toBeTrue(); + }); + + it("records the old and new framework versions on success", () => { + var f = buildFixture(); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + expect(result.oldVersion).toBe("4.0.0-SNAPSHOT+1687"); + expect(result.newVersion).toBe("4.0.1"); + }); + + it("backs up the existing vendor/wheels/ when doBackup is true", () => { + var f = buildFixture(); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, true); + expect(result.success).toBeTrue(); + expect(len(result.backupDir)).toBeGT(0); + expect(directoryExists(result.backupDir)).toBeTrue(); + expect(fileRead(result.backupDir & "/marker.txt")).toBe("old-framework"); + // And the live vendor/wheels/ has the new contents. + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("new-framework"); + }); + + it("uses a vendor/wheels.bak- naming pattern for the backup", () => { + var f = buildFixture(); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, true); + expect(reFindNoCase("/wheels\.bak-\d{8}-\d{6}", result.backupDir)).toBeGT(0); + }); + + it("honors a caller-reserved backupPath so pre-swap announcements match reality", () => { + // runUpgradeApply announces the backup destination BEFORE the + // swap (#3039 review) — the reserved path it prints must be the + // path the backup actually lands on. + var f = buildFixture(); + var reserved = upgrader.reserveBackupPath(f.vendorDir); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, true, reserved); + expect(result.success).toBeTrue(); + expect(result.backupDir).toBe(reserved); + expect(directoryExists(reserved)).toBeTrue(); + expect(fileRead(reserved & "/marker.txt")).toBe("old-framework"); + }); + + it("does NOT create a backup when doBackup is false", () => { + var f = buildFixture(); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + expect(result.success).toBeTrue(); + expect(result.backupDir).toBe(""); + // No sibling .bak-* directory should exist. + var sibs = directoryList(f.vendorParent, false, "name"); + for (var name in sibs) { + expect(reFindNoCase("^wheels\.bak-", name)).toBe(0); + } + }); + + it("returns an error when the source directory does not exist", () => { + var f = buildFixture(); + var bogusSource = f.root & "/does-not-exist"; + var result = upgrader.applyUpgrade(bogusSource, f.vendorDir, false); + expect(result.success).toBeFalse(); + expect(result.error).toInclude("Source"); + // And vendor/wheels/ is untouched. + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + }); + + it("returns an error when the source directory lacks a wheels.json/box.json marker", () => { + var f = buildFixture(); + fileDelete(f.sourceDir & "/wheels.json"); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + expect(result.success).toBeFalse(); + expect(result.error).toInclude("does not look like a Wheels framework"); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + }); + + it("refuses a source directory whose box.json belongs to a generic app", () => { + // #3039 review: any CFML project has a box.json — pointing + // WHEELS_FRAMEWORK_PATH (or a mangled install tree) at a + // random app must not vendor that app into the target. + var f = buildFixture(); + fileDelete(f.sourceDir & "/wheels.json"); + fileWrite(f.sourceDir & "/box.json", '{"name":"myapp","version":"1.0.0"}'); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + expect(result.success).toBeFalse(); + expect(result.error).toInclude("does not look like a Wheels framework"); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + }); + + it("refuses to swap when target dir exists but does not look like a Wheels framework", () => { + var f = buildFixture(writeTargetManifest = false); + // buildFixture skipped writing wheels.json — the target dir is + // now just a directory with marker.txt, so the sniff must fail. + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + expect(result.success).toBeFalse(); + expect(result.error).toInclude("does not look like a Wheels framework"); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + }); + + it("refuses when source and target resolve to the same directory", () => { + // Running the apply inside the wheels repo checkout itself + // resolves the bundled source to the very directory being + // replaced — the rename/delete would destroy the source + // mid-swap. The guard must fire before any destructive step. + var f = buildFixture(); + var result = upgrader.applyUpgrade(f.vendorDir, f.vendorDir, true); + expect(result.success).toBeFalse(); + expect(result.error).toInclude("same directory"); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + // No backup may exist — that would mean the rename ran first. + var sibs = directoryList(f.vendorParent, false, "name"); + for (var name in sibs) { + expect(reFindNoCase("^wheels\.bak-", name)).toBe(0); + } + }); + + it("refuses when the target lives inside the source directory", () => { + var f = buildFixture(); + // Make vendor/ itself sniff as a framework dir and use it as + // the source — the target vendor/wheels/ sits inside it. + fileWrite(f.vendorParent & "/wheels.json", '{"version":"9.9.9"}'); + var result = upgrader.applyUpgrade(f.vendorParent, f.vendorDir, false); + expect(result.success).toBeFalse(); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + }); + + it("refuses when the source lives inside the target directory", () => { + var f = buildFixture(); + var nested = f.vendorDir & "/sub"; + directoryCreate(nested, true, true); + fileWrite(nested & "/wheels.json", '{"version":"9.9.9"}'); + var result = upgrader.applyUpgrade(nested, f.vendorDir, false); + expect(result.success).toBeFalse(); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("old-framework"); + }); + + it("creates vendor/wheels/ from scratch when it does not already exist", () => { + var f = buildFixture(); + directoryDelete(f.vendorDir, true); + var result = upgrader.applyUpgrade(f.sourceDir, f.vendorDir, true); + expect(result.success).toBeTrue(); + expect(result.oldVersion).toBe(""); + expect(result.backupDir).toBe(""); + expect(fileRead(f.vendorDir & "/marker.txt")).toBe("new-framework"); + }); + + it("returns an error when the parent of vendorDir does not exist", () => { + var f = buildFixture(); + var bogusVendor = f.root & "/no-parent-here/wheels"; + var result = upgrader.applyUpgrade(f.sourceDir, bogusVendor, false); + expect(result.success).toBeFalse(); + expect(result.error).toInclude("Parent"); + }); + + it("throws CopyFailed naming the backup to restore when the copy fails after the rename", () => { + // An unreadable file inside the source makes directoryCopy + // blow up AFTER the backup rename already ran — the exact + // mid-swap failure the error contract exists for (#3039 + // review). POSIX-only simulation: Windows can't revoke read + // permission via File.setReadable, so skip there (the + // contract is exercised on every POSIX run). + var f = buildFixture(); + var blocker = f.sourceDir & "/unreadable.txt"; + fileWrite(blocker, "secret"); + if (!$makeUnreadable(blocker)) { + return; + } + var state = {caught = false, type = "", message = ""}; + try { + upgrader.applyUpgrade(f.sourceDir, f.vendorDir, true); + } catch (any e) { + state.caught = true; + state.type = e.type; + state.message = e.message; + } + $makeReadable(blocker); + + expect(state.caught).toBeTrue(); + expect(state.type).toBe("Wheels.FrameworkUpgrader.CopyFailed"); + // The rename ran before the copy, so the backup exists on + // disk and the message must name it (quoted) for the restore. + var backups = directoryList(f.vendorParent, false, "name", "wheels.bak-*"); + expect(arrayLen(backups)).toBe(1); + expect(state.message).toInclude(backups[1]); + expect(state.message).toInclude('"' & f.vendorDir & '"'); + expect(state.message).toInclude("partial state"); + }); + + it("CopyFailed with --nobackup says the old tree is gone and how to re-vendor", () => { + var f = buildFixture(); + var blocker = f.sourceDir & "/unreadable.txt"; + fileWrite(blocker, "secret"); + if (!$makeUnreadable(blocker)) { + return; + } + var state = {caught = false, type = "", message = ""}; + try { + upgrader.applyUpgrade(f.sourceDir, f.vendorDir, false); + } catch (any e) { + state.caught = true; + state.type = e.type; + state.message = e.message; + } + $makeReadable(blocker); + + expect(state.caught).toBeTrue(); + expect(state.type).toBe("Wheels.FrameworkUpgrader.CopyFailed"); + expect(state.message).toInclude("no backup exists"); + expect(state.message).toInclude("gone"); + expect(state.message).toInclude("wheels upgrade apply"); + }); + }); + } +} diff --git a/vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc b/vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc index a2cc79c1ab..b45711112d 100644 --- a/vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc +++ b/vendor/wheels/tests/specs/cli/UpgradeCommandHelpSpec.cfc @@ -1,16 +1,19 @@ /** - * Regression: cli/lucli/Module.cfc's `upgrade` command surface advertises an - * upgrader that doesn't exist. The main `showHelp()` summary line claims the - * command will "Upgrade the Wheels framework version in your project," while - * the runtime dispatcher only honours `wheels upgrade check [--to=]` - * — a read-only scanner that points users at `brew upgrade wheels` for the - * actual install. Users running `wheels upgrade --dry-run` or - * `wheels upgrade --to=4.0.0` (both flags implied by the misleading summary) - * land on a terse usage line and are left guessing. + * Regression: cli/lucli/Module.cfc's `upgrade` command surface must match + * what the command actually does. * - * Issue #2629. The fix is to align every public-facing description of - * `wheels upgrade` with the scanner-only reality: the showHelp summary, the - * docblock hint, and the function's own usage output. + * History: issue #2629 fixed the original drift — the help claimed an + * upgrader while the runtime only honoured the read-only + * `wheels upgrade check` scanner — by rewording every public-facing + * description down to the scanner-only reality. Issue #3035 then added + * the framework swap (replacing the app's vendor/wheels/ with the CLI's + * bundled framework, backup first), and the #3039 review put it behind + * the explicit `apply` verb: bare `wheels upgrade` prints usage and never + * mutates, `wheels upgrade apply` performs the swap, `check` keeps the + * read-only scan. The same alignment rules apply: the showHelp() summary, + * the docblock hint, and the usage output must all advertise BOTH verbs — + * without resurrecting the old "this command is read-only" claims and + * without claiming the bare verb applies anything. */ component extends="wheels.WheelsTest" { @@ -25,53 +28,51 @@ component extends="wheels.WheelsTest" { expect(fileExists(modulePath)).toBeTrue("Missing file: " & modulePath); }); - it("showHelp() summary line no longer claims to perform an upgrade", () => { + it("showHelp() summary line advertises the upgrade capability", () => { var source = fileRead(modulePath); - // The legacy phrasing implies the command performs the - // upgrade itself. It doesn't — it's a read-only scanner. - expect(source contains "upgrade Upgrade the Wheels framework version in your project").toBeFalse( - "showHelp() still summarises `wheels upgrade` as an upgrader. " - & "The command is read-only — describe it as scanning for breaking changes." + // Since #3035 the command can perform the swap, so the summary + // must say so (the #2629-era scanner-only summary is stale). + expect(source contains "Upgrade the Wheels framework in your app").toBeTrue( + "showHelp() should summarise `wheels upgrade` as upgrading the framework " + & "copy inside the app (vendor/wheels/) — that's what the `apply` verb does " + & "as of ##3035." ); }); - it("showHelp() summary line describes the command as a scanner", () => { + it("usage output still advertises the read-only check subcommand", () => { var source = fileRead(modulePath); - // One of these phrasings should be present in the - // showHelp() summary block for the `upgrade` entry. - var summariesScanner = source contains "Scan for breaking changes before upgrading" - || source contains "Check for breaking changes before upgrading"; - - expect(summariesScanner).toBeTrue( - "showHelp() should describe `wheels upgrade` as scanning or checking for " - & "breaking changes — that's what the command actually does. Add a scanner-" - & "oriented summary line for the `upgrade` entry." + expect(source contains "wheels upgrade check").toBeTrue( + "upgrade() should advertise the `check` subcommand in its usage output " + & "so users can preview breaking changes before applying." ); }); - it("upgrade() usage output mentions the required `check` subcommand", () => { + it("usage output advertises the explicit apply verb", () => { var source = fileRead(modulePath); - expect(source contains "wheels upgrade check").toBeTrue( - "upgrade() should advertise the `check` subcommand in its usage output " - & "so users who run `wheels upgrade --dry-run` or `wheels upgrade --to=...` " - & "discover the right invocation." + // #3039 review: the swap is behind `wheels upgrade apply` — + // bare `wheels upgrade` prints usage and never mutates, so + // the help must steer users at the explicit verb. + expect(source contains "wheels upgrade apply").toBeTrue( + "upgrade() should advertise the `apply` subcommand in its usage output — " + & "the swap requires the explicit verb as of the ##3039 review " + & "(bare `wheels upgrade` is a usage printout, not the apply path)." ); }); - it("upgrade() usage output points users at the real upgrade path", () => { + it("usage output points at the package manager for the CLI binary itself", () => { var source = fileRead(modulePath); expect(source contains "brew upgrade wheels").toBeTrue( - "upgrade() should tell users that the actual framework upgrade is performed " - & "by `brew upgrade wheels` (or the equivalent package manager), not by " - & "this command." + "upgrade() should tell users that the CLI binary is upgraded by " + & "`brew upgrade wheels` (or the equivalent package manager) — apply mode " + & "only swaps the framework copy the CLI bundles." ); }); - it("upgrade() usage output explicitly notes that --dry-run is not supported", () => { + it("usage output explicitly notes that --dry-run is not supported", () => { var source = fileRead(modulePath); // Either an explicit `--dry-run is not supported` line, or @@ -83,13 +84,24 @@ component extends="wheels.WheelsTest" { || source contains "no --dry-run"; expect(mentionsDryRunGap).toBeTrue( - "The usage block in upgrade() should call out that `--dry-run` is not " - & "supported. The flag is implied by the misleading legacy summary; " - & "naming the gap in the usage output is what keeps users unstuck." + "The help surface should call out that `--dry-run` is not supported — " + & "`wheels upgrade check` is the read-only preview." ); }); - it("upgrade() docblock hint matches the scanner-only reality", () => { + it("the apply path exists with the backup convention", () => { + var source = fileRead(modulePath); + + expect(source contains "runUpgradeApply").toBeTrue( + "Module.cfc should dispatch the `apply` verb to runUpgradeApply() (##3035/##3039)." + ); + expect(source contains "wheels.bak-").toBeTrue( + "The apply path should reference the vendor/wheels.bak- " + & "backup convention so the help/recovery output stays truthful." + ); + }); + + it("upgrade() docblock hint matches the apply-first reality", () => { var source = fileRead(modulePath); // Anchor on the function declaration first, then walk @@ -120,9 +132,20 @@ component extends="wheels.WheelsTest" { var hintLen = (hintEnd > hintStart) ? (hintEnd - hintStart) : (len(source) - hintStart + 1); var hintLine = mid(source, hintStart, hintLen); - expect(reFindNoCase("\bupgrade\s+the\s+wheels\s+framework\b", hintLine) > 0).toBeFalse( - "upgrade() hint still promises to `upgrade the Wheels framework`. " - & "It's a scanner — phrase the hint as `Scan ...` or `Check ...`." + // Inverted from the #2629-era assertion: the hint MUST + // now promise the upgrade (that's what `apply` does) + // and surface both explicit verbs — check as the + // read-only scan, apply as the swap (#3039 review). + expect(reFindNoCase("\bupgrade\s+the\s+wheels\s+framework\b", hintLine) > 0).toBeTrue( + "upgrade() hint should advertise the upgrade capability — " + & "`wheels upgrade apply` performs the framework swap as of ##3035/##3039." + ); + expect(findNoCase("check", hintLine) > 0).toBeTrue( + "upgrade() hint should still mention the read-only `check` scan." + ); + expect(findNoCase("apply", hintLine) > 0).toBeTrue( + "upgrade() hint should mention the explicit `apply` verb — bare " + & "`wheels upgrade` no longer performs the swap (##3039 review)." ); } }