From 72ed1f112dc982bd2f7bf2494163a68c711b7419 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 09:19:36 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20[nomad-step-0]=20S0.1-fix=20=E2=80=94=20?= =?UTF-8?q?bin/disinto=20swallows=20--backend=3Dnomad=20as=20repo=5Furl=20?= =?UTF-8?q?positional=20(#835)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: disinto_init() consumed $1 as repo_url before the argparse loop ran, so `disinto init --backend=nomad --empty` had --backend=nomad swallowed into repo_url, backend stayed at its "docker" default, and the --empty validation then produced the nonsense "--empty is only valid with --backend=nomad" error — flagged during S0.1 end-to-end verification on a fresh LXC. nomad backend takes no positional anyway; the LXC already has the repo cloned by the operator. Change: only consume $1 as repo_url if it doesn't start with "--", then defer the "repo URL required" check to after argparse (so the docker path still errors with a helpful message on a missing positional, not "Unknown option: --backend=docker"). Verified acceptance criteria: 1. init --backend=nomad --empty → dispatches to nomad 2. init --backend=nomad --empty --dry-run → 9-step plan, exit 0 3. init → docker path unchanged 4. init → "repo URL required" 5. init --backend=docker → "repo URL required" (not "Unknown option") 6. shellcheck clean Tests: 4 new regression cases in tests/disinto-init-nomad.bats covering flag-first nomad invocation (both --flag=value and --flag value forms), no-args docker default, and --backend=docker missing-positional error path. Full suite: 10/10 pass. Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/disinto | 27 ++++++++++++++++++------ tests/disinto-init-nomad.bats | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/bin/disinto b/bin/disinto index 12072d1..4f06b5e 100755 --- a/bin/disinto +++ b/bin/disinto @@ -707,13 +707,18 @@ _disinto_init_nomad() { } disinto_init() { - local repo_url="${1:-}" - if [ -z "$repo_url" ]; then - echo "Error: repo URL required" >&2 - echo "Usage: disinto init " >&2 - exit 1 + # Only consume $1 as repo_url if it looks like a positional arg (not a + # flag). The nomad backend (#835) takes no positional — the LXC already + # has the repo cloned by the operator, and repo_url is a docker-backend + # concept. Eagerly consuming `--backend=nomad` as repo_url produced the + # nonsense "--empty is only valid with --backend=nomad" error seen in + # S0.1 end-to-end testing on a fresh LXC. Defer the "repo URL required" + # check to after argparse, where we know the backend. + local repo_url="" + if [ $# -gt 0 ] && [[ "$1" != --* ]]; then + repo_url="$1" + shift fi - shift # Parse flags local branch="" repo_root="" ci_id="0" auto_yes=false forge_url_flag="" bare=false rotate_tokens=false use_build=false dry_run=false backend="docker" empty=false @@ -741,6 +746,16 @@ disinto_init() { *) echo "Error: invalid --backend value '${backend}' (expected: docker|nomad)" >&2; exit 1 ;; esac + # Docker backend requires a repo_url positional; nomad doesn't use one. + # This check must run *after* argparse so `--backend=docker` (with no + # positional) errors with a helpful message instead of the misleading + # "Unknown option: --backend=docker". + if [ "$backend" = "docker" ] && [ -z "$repo_url" ]; then + echo "Error: repo URL required" >&2 + echo "Usage: disinto init [options]" >&2 + exit 1 + fi + # --empty is nomad-only today (the docker path has no concept of an # "empty cluster"). Reject explicitly rather than letting it silently # do nothing on --backend=docker. diff --git a/tests/disinto-init-nomad.bats b/tests/disinto-init-nomad.bats index 16315dc..5b2648b 100644 --- a/tests/disinto-init-nomad.bats +++ b/tests/disinto-init-nomad.bats @@ -104,3 +104,42 @@ setup_file() { [ "$status" -ne 0 ] [[ "$output" == *"--empty is only valid with --backend=nomad"* ]] } + +# ── Positional vs flag-first invocation (#835) ─────────────────────────────── +# +# Before the #835 fix, disinto_init eagerly consumed $1 as repo_url *before* +# argparse ran. That swallowed `--backend=nomad` as a repo_url and then +# complained that `--empty` required a nomad backend — the nonsense error +# flagged during S0.1 end-to-end verification. The cases below pin the CLI +# to the post-fix contract: the nomad path accepts flag-first invocation, +# the docker path still errors helpfully on a missing repo_url. + +@test "disinto init --backend=nomad --empty --dry-run (no positional) dispatches to nomad" { + run "$DISINTO_BIN" init --backend=nomad --empty --dry-run + [ "$status" -eq 0 ] + [[ "$output" == *"nomad backend: --empty (cluster-up only, no jobs)"* ]] + [[ "$output" == *"[dry-run] Step 1/9: install nomad + vault binaries"* ]] + # The bug symptom must be absent — backend was misdetected as docker + # when --backend=nomad got swallowed as repo_url. + [[ "$output" != *"--empty is only valid with --backend=nomad"* ]] +} + +@test "disinto init --backend nomad --dry-run (space-separated, no positional) dispatches to nomad" { + run "$DISINTO_BIN" init --backend nomad --dry-run + [ "$status" -eq 0 ] + [[ "$output" == *"nomad backend: default"* ]] + [[ "$output" == *"[dry-run] Step 1/9: install nomad + vault binaries"* ]] +} + +@test "disinto init (no args) still errors with 'repo URL required'" { + run "$DISINTO_BIN" init + [ "$status" -ne 0 ] + [[ "$output" == *"repo URL required"* ]] +} + +@test "disinto init --backend=docker (no positional) errors with 'repo URL required', not 'Unknown option'" { + run "$DISINTO_BIN" init --backend=docker + [ "$status" -ne 0 ] + [[ "$output" == *"repo URL required"* ]] + [[ "$output" != *"Unknown option"* ]] +} -- 2.49.1