diff --git a/CHANGELOG.md b/CHANGELOG.md index 2196830..03953d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to netbox-super-cli are tracked here. Format follows [Keep a ## Unreleased +### Fixed + +- **`--branch` after a subcommand no longer switches branches** ([#170]). It is + a global option only before the subcommand. After it, `--branch` belongs to + that command (for example a plugin's `branch` filter) and is left alone. +- **Branch failures while loading `--saved` filters honour `-o json`** ([#173]). + They now emit the branch error's structured envelope (with its usual exit + code) in the requested format instead of falling through to the catch-all + table renderer. `nsc --branch X ` (e.g. `profiles list`) is + also recognised as a meta command again instead of bootstrapping the schema. + ## v1.8.0 — 2026-10-22 Minor release. Adds NetBox Branching support (`--branch`, `NSC_BRANCH`, @@ -91,6 +102,8 @@ notices, and a fix for `nsc login` reporting the wrong user. [#1]: https://github.com/thomaschristory/netbox-super-cli/issues/1 [#166]: https://github.com/thomaschristory/netbox-super-cli/issues/166 +[#170]: https://github.com/thomaschristory/netbox-super-cli/issues/170 +[#173]: https://github.com/thomaschristory/netbox-super-cli/issues/173 [#150]: https://github.com/thomaschristory/netbox-super-cli/issues/150 [#151]: https://github.com/thomaschristory/netbox-super-cli/issues/151 diff --git a/docs/guides/branching.md b/docs/guides/branching.md index 8de87cb..d402550 100644 --- a/docs/guides/branching.md +++ b/docs/guides/branching.md @@ -34,6 +34,11 @@ The active branch comes from the first of these that is set: `--branch main` or `NSC_BRANCH=main` forces main for that invocation even when the profile has a sticky branch. +`--branch` is a global option and must come **before** the subcommand +(`nsc --branch X dcim sites list`). After the subcommand, `--branch` belongs to +that command, for example the Branching plugin's own `changes list` filter, and +doesn't switch branches. + `nsc branch use` needs a saved profile to store the branch on. With only `NSC_URL`/`NSC_TOKEN` set (no profile), use `NSC_BRANCH` or `--branch`. diff --git a/nsc/cli/app.py b/nsc/cli/app.py index 669c26d..012ed16 100644 --- a/nsc/cli/app.py +++ b/nsc/cli/app.py @@ -73,19 +73,27 @@ def _extract_global_overrides(args: list[str]) -> CLIOverrides: "-o": "output", "--branch": "branch", } + # Only `--branch` is root-only: dynamic commands may define their own + # `--branch` filter (e.g. the Branching plugin's changes list), which must + # not switch the whole invocation into a branch. + root_only = {"--branch"} + seen_subcommand = False i = 0 while i < len(args): a = args[i] if a.startswith("--") and "=" in a: flag, _, value = a.partition("=") - if flag in two_arg: + if flag in two_arg and not (seen_subcommand and flag in root_only): kwargs[two_arg[flag]] = value i += 1 continue if a in two_arg and i + 1 < len(args): - kwargs[two_arg[a]] = args[i + 1] + if not (seen_subcommand and a in root_only): + kwargs[two_arg[a]] = args[i + 1] i += 2 continue + if not a.startswith("-"): + seen_subcommand = True if a == "--insecure": kwargs["insecure"] = True i += 1 @@ -134,6 +142,7 @@ def _first_non_option(args: list[str]) -> str | None: "--schema", "--output", "-o", + "--branch", } for a in args: if skip_next: diff --git a/nsc/cli/registration.py b/nsc/cli/registration.py index 225351b..515880e 100644 --- a/nsc/cli/registration.py +++ b/nsc/cli/registration.py @@ -13,6 +13,7 @@ # different, rejected type on the vendored-click line). See issue #82. from typer._types import TyperChoice as ChoiceType +from nsc.branching.errors import BranchError from nsc.cli.handlers import ( handle_create, handle_custom_action, @@ -22,7 +23,7 @@ handle_list, handle_update, ) -from nsc.cli.runtime import RuntimeContext +from nsc.cli.runtime import RuntimeContext, emit_envelope, map_error from nsc.config.models import OutputFormat from nsc.http.errors import NetBoxAPIError, NetBoxClientError from nsc.model.command_model import ( @@ -196,14 +197,20 @@ def impl(**kwargs: Any) -> None: explicit_filters = [ (item.split("=", 1)[0], item.split("=", 1)[1]) for item in filters_raw if "=" in item ] + try: + filters = _resolve_saved_filters( + ctx, tag_name, resource_name, operation.path, saved_name, explicit_filters, kwargs + ) + except BranchError as exc: + env = map_error(exc, branch=ctx.branch.name if ctx.branch else None) + fmt = OutputFormat(output) if output else ctx.output_format + raise typer.Exit(emit_envelope(env, output_format=fmt, color=ctx.color_stderr)) from exc update: dict[str, Any] = { "compact": compact, "columns_override": columns_csv.split(",") if columns_csv else None, "limit": limit, "fetch_all": fetch_all, - "filters": _resolve_saved_filters( - ctx, tag_name, resource_name, operation.path, saved_name, explicit_filters, kwargs - ), + "filters": filters, } if output: update["output_format"] = OutputFormat(output) diff --git a/tests/cli/test_branch_commands.py b/tests/cli/test_branch_commands.py index 06cb7c5..ff2558a 100644 --- a/tests/cli/test_branch_commands.py +++ b/tests/cli/test_branch_commands.py @@ -273,3 +273,19 @@ def test_api_error_envelope_names_active_branch(httpx2_mock: respx.Router, _home result = _invoke("dcim", "sites", "list", "--output", "json") assert result.exit_code == 4, result.output assert json.loads(result.stdout)["details"]["branch"] == "my-change" + + +@pytest.mark.httpx2(assert_all_called=False) +def test_saved_filter_branch_failure_honours_json_output( + httpx2_mock: respx.Router, _home: Path +) -> None: + _mock_schema(httpx2_mock) + _set_profile_branch(_home, "my-change") + _mock_lookup(httpx2_mock, status={"value": "merged"}) + sites = httpx2_mock.get(url__startswith=f"{NB}/api/dcim/sites/").respond(200, json=page()) + result = _invoke("dcim", "sites", "list", "--saved", "mine", "--output", "json") + assert result.exit_code == 12, (result.exit_code, result.output, result.exception) + envelope = json.loads(result.stdout) + assert envelope["type"] == "config" + assert envelope["details"]["branch"] == "my-change" + assert not sites.called diff --git a/tests/cli/test_runtime_branch.py b/tests/cli/test_runtime_branch.py index 34c530f..345090b 100644 --- a/tests/cli/test_runtime_branch.py +++ b/tests/cli/test_runtime_branch.py @@ -13,7 +13,7 @@ BranchNotReadyError, ) from nsc.cli import globals as globals_mod -from nsc.cli.app import _extract_global_overrides +from nsc.cli.app import _extract_global_overrides, _first_non_option from nsc.cli.globals import GlobalState, build_runtime_context from nsc.cli.runtime import BranchSelection, CLIOverrides, map_error, resolve_branch_selection from nsc.config.loader import load_config @@ -133,3 +133,25 @@ def test_map_error_job_failed_details() -> None: env = map_error(BranchJobFailedError("x", {"id": 7, "error": "nope"})) assert env.details["job_id"] == 7 assert env.details["job_error"] == "nope" + + +def test_branch_after_subcommand_is_not_global() -> None: + # A dynamic command's own --branch filter (e.g. the plugin's changes list) + # must not switch the whole invocation into a branch. + args = ["plugins", "branching", "changes", "list", "--branch", "x"] + assert _extract_global_overrides(args).branch is None + assert _extract_global_overrides([*args[:-2], "--branch=x"]).branch is None + + +def test_branch_before_subcommand_is_global_and_other_flags_still_scan() -> None: + overrides = _extract_global_overrides( + ["--profile", "p", "--branch", "x", "dcim", "sites", "list", "--output", "json"] + ) + assert overrides.branch == "x" + assert overrides.profile == "p" + assert overrides.output == "json" + + +def test_first_non_option_skips_branch_value() -> None: + assert _first_non_option(["--branch", "x", "profiles", "list"]) == "profiles" + assert _first_non_option(["--branch=x", "profiles", "list"]) == "profiles"