From 1b76de1de44f9174c4fc8e5d84d342892b6a7206 Mon Sep 17 00:00:00 2001 From: stagknee <241821830+stagknee@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:50:14 -0500 Subject: [PATCH] fix: mxcli new warns instead of failing when the devcontainer binary can't be fetched (closes #1365) Step 7 of `mxcli new` exited 1 when the Linux mxcli download failed, though steps 1-6 had already produced a complete project. A dev build reports 0.1.0, whose tag v0.1.0 has no release, so every `new` from a local build 404'd. A failed download is now a warning, and a build with no matching release skips the download with a note. Co-Authored-By: Claude Sonnet 5.5 --- ...ts-1-step-7-linux-binary-download-404.json | 1 + CHANGELOG.md | 1 + cmd/mxcli/cmd_new.go | 35 ++++- cmd/mxcli/cmd_new_devcontainer_test.go | 128 ++++++++++++++++++ cmd/mxcli/cmd_new_test.go | 4 +- cmd/mxcli/setup.go | 20 +++ 6 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-mxcli-new-exits-1-step-7-linux-binary-download-404.json create mode 100644 cmd/mxcli/cmd_new_devcontainer_test.go diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-mxcli-new-exits-1-step-7-linux-binary-download-404.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-mxcli-new-exits-1-step-7-linux-binary-download-404.json new file mode 100644 index 0000000000..8d49e69ee3 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-mxcli-new-exits-1-step-7-linux-binary-download-404.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-09", "symptom": "`mxcli new` from a locally built mxcli on Windows/macOS ends with `Error: could not download Linux mxcli binary for devcontainer: … HTTP 404 from …/releases/download/v0.1.0/mxcli-linux-amd64` and exit status 1, although steps 1-6 had already produced a complete project; no network or blocked GitHub assets did the same", "cause": "Step 7 fetched the Linux binary with `mxcliReleaseTag()` and `os.Exit(1)` on any error. A build without `-X main.Version` reports the default version `0.1.0`, so the tag is `v0.1.0`, which was never released: every `mxcli new` from a dev build was a guaranteed 404. Step 6 (first build) already treated its own failure as a warning, so the project was usable either way", "file": "`cmd/mxcli/cmd_new.go` (`fetchDevcontainerMxcli`, step 7) + `cmd/mxcli/setup.go` (`hasPublishedRelease`)", "insight": "By the last step of a multi-step scaffold the deliverable exists; a best-effort step must warn, not exit. Dev builds are detected by `Version == \"\"` (no ldflags) or a tag that is neither `nightly` nor `vX.Y.Z` (`make build` outside git stamps `dev` or a bare hash); `git describe` output past a tag (`v0.24.0-888-gHASH`) still maps to the real `v0.24.0` release, so it keeps downloading. The step is a function with the downloader in a package var (`downloadDevcontainerBinary`), because the inline `os.Exit` in the cobra RunE could not be tested. `mxcliReleaseTag()` is unchanged: `setup mxcli` also calls it and an explicit `--tag` is the way out for a dev build. Issue #1365", "refs": ["mendixlabs/mxcli#1365"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 68988b1d9a..631b370ed6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`mxcli new` warns instead of failing when the devcontainer binary can't be fetched** (mendixlabs/mxcli#1365) — on Windows and macOS, step 7 downloads a Linux mxcli for the devcontainer and exited 1 when that failed, although steps 1–6 had already produced a complete project. A locally built mxcli reports version `0.1.0`, whose release tag `v0.1.0` does not exist, so every `mxcli new` from a development build ended in `HTTP 404` and exit status 1; no network or blocked GitHub assets did the same. A failed download is now a warning (the project is usable; `mxcli setup mxcli --output ./mxcli` fetches the binary later) and `new` continues to its summary and exits 0. A development build (no `-X main.Version`, or a version that is not `vX.Y.Z` / `nightly`) skips the download with a note instead of attempting a guaranteed 404. Release, nightly and `git describe` builds download as before; the Linux hard-link/copy path is unchanged. - **A navigation sub-menu no longer keeps the action of the page item it replaces** (mendixlabs/mxcli#1341) — turning menu item `'Work'` into sub-menu `'Work'` with `create or replace navigation` (or `create or modify menu`) paired the two by caption and carried the page item's `Forms$FormAction` onto the sub-menu, reporting it as an action "MDL cannot express"; `mx check` then failed with CE0548 "Items with subitems cannot have an action themselves." A sub-menu is now always written with no action (measured on 11.14.0). - **`mxcli test --attach` and the dev loop's "already serving" detection see a live app on Windows** (mendixlabs/mxcli#1284) — both read a pid from a handshake file (`.mxcli/test-endpoint.json`, `.mxcli/run-local.json`) and tested it with `Signal(0)`, which Go supports only as Kill on Windows, so every pid read as dead: `test --attach` always refused with "the app that published … is no longer running" while the app was up, and a command that looks for a dev loop already serving the project (the recompile warning, for one) never found it. Liveness is now `internal/procalive.Alive`, which uses OpenProcess and WaitForSingleObject on Windows (the check `run --local` already used for mxbuild) and `Signal(0)` elsewhere. No change on Linux or macOS. - **`run stop` on Windows no longer reports a run that has shut down as still alive** — it printed "failed: 1 process(es) of pid N's run are still alive" and exited 1 for a run that had stopped, because its liveness check counted any pid Windows could still open as alive, and an exited process stays openable while its parent holds a handle to it. It now uses the same check as `test --attach` (above), and reports "stopped: pid N". diff --git a/cmd/mxcli/cmd_new.go b/cmd/mxcli/cmd_new.go index 4f967889d6..e2704323a9 100644 --- a/cmd/mxcli/cmd_new.go +++ b/cmd/mxcli/cmd_new.go @@ -4,6 +4,7 @@ package main import ( "fmt" + "io" "os" "os/exec" "path/filepath" @@ -300,13 +301,7 @@ Examples: mxcliBinPath := filepath.Join(absDir, "mxcli") if runtime.GOOS != "linux" { // Running on Windows/macOS — download the Linux binary for devcontainer - tag := mxcliReleaseTag() - fmt.Printf(" Downloading Linux mxcli (%s) for devcontainer...\n", tag) - if err := downloadMxcliBinary("mendixlabs/mxcli", tag, "linux", "amd64", mxcliBinPath, os.Stdout); err != nil { - fmt.Fprintf(os.Stderr, "Error: could not download Linux mxcli binary for devcontainer: %v\n", err) - fmt.Fprintln(os.Stderr, " Run 'mxcli setup mxcli --output ./mxcli' inside the project directory to fix this.") - os.Exit(1) - } + fetchDevcontainerMxcli(mxcliBinPath, os.Stdout, os.Stderr) } else { // Running on Linux — link ourselves into the project. Prefer a hard link: // it shares the inode (no ~111MB duplicated per project on the same @@ -402,3 +397,29 @@ func init() { rootCmd.AddCommand(newCmd) } + +// downloadDevcontainerBinary is the downloader step 7 of `mxcli new` uses. +// It is a variable so tests can stand in for the network. +var downloadDevcontainerBinary = downloadMxcliBinary + +// fetchDevcontainerMxcli downloads the Linux mxcli into the project for the +// devcontainer to use (step 7 of `mxcli new` on Windows/macOS). +// +// By step 7 the project is complete, so nothing here may fail the command: +// a development build has no release to download (skipped with a note), and a +// failed download — no network, blocked GitHub assets — is a warning, like a +// failed first build in step 6 (mendixlabs/mxcli#1365). +func fetchDevcontainerMxcli(mxcliBinPath string, out, errOut io.Writer) { + if !hasPublishedRelease() { + fmt.Fprintf(out, " This is a development build (version %s) with no matching release, so no Linux mxcli was downloaded.\n", version) + fmt.Fprintln(out, " For the devcontainer, run 'mxcli setup mxcli --tag --output ./mxcli' inside the project directory.") + return + } + tag := mxcliReleaseTag() + fmt.Fprintf(out, " Downloading Linux mxcli (%s) for devcontainer...\n", tag) + if err := downloadDevcontainerBinary("mendixlabs/mxcli", tag, "linux", "amd64", mxcliBinPath, out); err != nil { + fmt.Fprintf(errOut, " Warning: could not download the Linux mxcli binary for the devcontainer: %v\n", err) + fmt.Fprintln(errOut, " The project is usable. Run 'mxcli setup mxcli --output ./mxcli' inside the project") + fmt.Fprintln(errOut, " directory to fetch it later.") + } +} diff --git a/cmd/mxcli/cmd_new_devcontainer_test.go b/cmd/mxcli/cmd_new_devcontainer_test.go new file mode 100644 index 0000000000..052b6cf641 --- /dev/null +++ b/cmd/mxcli/cmd_new_devcontainer_test.go @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "errors" + "io" + "strings" + "testing" +) + +// withBuildVersion sets the build's version variables for one test: ld is what +// -X main.Version would have set ("" for a plain `go build`), v is the +// package default the rest of the code reads. +func withBuildVersion(t *testing.T, ld, v string) { + t.Helper() + oldLD, oldV := Version, version + Version, version = ld, v + t.Cleanup(func() { Version, version = oldLD, oldV }) +} + +// stubDevcontainerDownload replaces the downloader for one test and returns a +// pointer to the number of times it was called. +func stubDevcontainerDownload(t *testing.T, fn func(tag, outPath string) error) *int { + t.Helper() + old := downloadDevcontainerBinary + calls := 0 + downloadDevcontainerBinary = func(repo, tag, targetOS, targetArch, outputPath string, w io.Writer) error { + calls++ + return fn(tag, outputPath) + } + t.Cleanup(func() { downloadDevcontainerBinary = old }) + return &calls +} + +// TestFetchDevcontainerMxcli_DownloadErrorIsAWarning is the #1365 regression: +// steps 1-6 of `mxcli new` have already produced a complete project, so a +// failed fetch of the Linux binary must not turn the whole command into a +// failure. The function has to return normally with a warning on stderr and +// the hint for fixing it by hand. +func TestFetchDevcontainerMxcli_DownloadErrorIsAWarning(t *testing.T) { + withBuildVersion(t, "v0.25.0", "v0.25.0") + calls := stubDevcontainerDownload(t, func(string, string) error { + return errors.New("HTTP 404") + }) + + var out, errOut bytes.Buffer + fetchDevcontainerMxcli("/proj/mxcli", &out, &errOut) + + if *calls != 1 { + t.Fatalf("expected one download attempt, got %d", *calls) + } + got := errOut.String() + if !strings.Contains(got, " Warning: could not download the Linux mxcli binary for the devcontainer: HTTP 404") { + t.Errorf("stderr lacks the warning:\n%s", got) + } + if strings.Contains(got, "Error:") { + t.Errorf("a failed fetch must not be reported as an Error:\n%s", got) + } + if !strings.Contains(got, "mxcli setup mxcli --output ./mxcli") { + t.Errorf("stderr lacks the manual-fix hint:\n%s", got) + } +} + +// TestFetchDevcontainerMxcli_DevBuildSkipsDownload: a locally built mxcli +// reports version 0.1.0, whose release tag (v0.1.0) does not exist. Attempting +// the download is a guaranteed 404, so it is skipped with a note instead. +func TestFetchDevcontainerMxcli_DevBuildSkipsDownload(t *testing.T) { + for _, tc := range []struct{ name, ld, v string }{ + {"plain go build", "", "0.1.0"}, + {"make build without git", "dev", "dev"}, + {"make build, untagged checkout", "4ba1495f2", "4ba1495f2"}, + } { + t.Run(tc.name, func(t *testing.T) { + withBuildVersion(t, tc.ld, tc.v) + calls := stubDevcontainerDownload(t, func(string, string) error { return nil }) + + var out, errOut bytes.Buffer + fetchDevcontainerMxcli("/proj/mxcli", &out, &errOut) + + if *calls != 0 { + t.Fatalf("dev build must not attempt a download, got %d call(s)", *calls) + } + if !strings.Contains(out.String(), "development build") { + t.Errorf("stdout lacks the dev-build note:\n%s", out.String()) + } + if !strings.Contains(out.String(), "mxcli setup mxcli --tag") { + t.Errorf("stdout lacks the hint to fetch a release by tag:\n%s", out.String()) + } + if errOut.Len() != 0 { + t.Errorf("a skipped download is not a warning, stderr = %q", errOut.String()) + } + }) + } +} + +// TestFetchDevcontainerMxcli_SuccessUsesReleaseTag: a release build downloads +// the matching tag into the project and prints no warning. +func TestFetchDevcontainerMxcli_SuccessUsesReleaseTag(t *testing.T) { + for _, tc := range []struct{ name, ld, v, wantTag string }{ + {"release", "v0.25.0", "v0.25.0", "v0.25.0"}, + {"nightly", "nightly-20261002-4ba1495f2", "nightly-20261002-4ba1495f2", "nightly"}, + {"git describe past a tag", "v0.24.0-888-g4ba1495f2", "v0.24.0-888-g4ba1495f2", "v0.24.0"}, + } { + t.Run(tc.name, func(t *testing.T) { + withBuildVersion(t, tc.ld, tc.v) + var gotTag, gotPath string + calls := stubDevcontainerDownload(t, func(tag, outPath string) error { + gotTag, gotPath = tag, outPath + return nil + }) + + var out, errOut bytes.Buffer + fetchDevcontainerMxcli("/proj/mxcli", &out, &errOut) + + if *calls != 1 || gotTag != tc.wantTag || gotPath != "/proj/mxcli" { + t.Fatalf("download calls=%d tag=%q path=%q, want 1 / %q / /proj/mxcli", *calls, gotTag, gotPath, tc.wantTag) + } + if !strings.Contains(out.String(), "Downloading Linux mxcli ("+tc.wantTag+") for devcontainer") { + t.Errorf("stdout lacks the download line:\n%s", out.String()) + } + if errOut.Len() != 0 { + t.Errorf("success must not write to stderr, got %q", errOut.String()) + } + }) + } +} diff --git a/cmd/mxcli/cmd_new_test.go b/cmd/mxcli/cmd_new_test.go index bc75a0eed7..917707ca8a 100644 --- a/cmd/mxcli/cmd_new_test.go +++ b/cmd/mxcli/cmd_new_test.go @@ -11,8 +11,8 @@ import ( ) // TestDownloadMxcliBinary_HTTP404ReturnsError verifies that a 404 from the -// release server is surfaced as an error. This exercises the path in -// cmd_new.go step 4 that must exit 1 when the download fails. +// release server is surfaced as an error. fetchDevcontainerMxcli turns this +// error into a warning in step 7 of cmd_new.go. func TestDownloadMxcliBinary_HTTP404ReturnsError(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) diff --git a/cmd/mxcli/setup.go b/cmd/mxcli/setup.go index 2f0788a4cd..3d4777478a 100644 --- a/cmd/mxcli/setup.go +++ b/cmd/mxcli/setup.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "os" + "regexp" "runtime" "strings" @@ -236,6 +237,25 @@ func mxcliReleaseTag() string { return v } +// releaseTagPattern matches a tag mxcliReleaseTag can produce for a published +// release: "vX.Y.Z" (nightly builds map to the literal "nightly" tag). +var releaseTagPattern = regexp.MustCompile(`^v\d+\.\d+\.\d+$`) + +// hasPublishedRelease reports whether the running binary has a matching +// release on GitHub to download from. A development build does not: a plain +// `go build` has no -X main.Version and reports the default "0.1.0" (so +// mxcliReleaseTag says v0.1.0, which was never released), and `make build` +// outside a git checkout stamps "dev" or a bare commit hash. Only a build +// stamped by `make release` (vX.Y.Z, vX.Y.Z-N-gHASH or nightly-...) maps to +// a real tag. +func hasPublishedRelease() bool { + if Version == "" { + return false + } + tag := mxcliReleaseTag() + return tag == "nightly" || releaseTagPattern.MatchString(tag) +} + // downloadMxcliBinary downloads the mxcli binary for the given OS/arch from // GitHub releases and writes it to outputPath with executable permissions. func downloadMxcliBinary(repo, tag, targetOS, targetArch, outputPath string, w io.Writer) error {