From ff7084821d0907015fd0a47a331f336bbec3d514 Mon Sep 17 00:00:00 2001 From: Thomas Christory Date: Fri, 23 Oct 2026 15:54:30 +0200 Subject: [PATCH 1/2] fix(branching): ignore the profile's sticky branch when --url targets another server (#171) resolve_branch_selection applied the profile's branch even when --url or NSC_URL pointed at a different NetBox, failing with not_found there. The profile branch is now dropped when the URL override differs from the profile URL (normalised; trailing slash and host case ignored). --branch and NSC_BRANCH still apply. Reuses _select_base_profile instead of duplicating the profile lookup. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 ++++++++ docs/guides/branching.md | 4 ++++ docs/guides/managing-profiles.md | 3 ++- nsc/cli/runtime.py | 17 ++++++++++++++--- tests/cli/test_runtime_branch.py | 30 ++++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e7982d..97198a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to netbox-super-cli are tracked here. Format follows [Keep a ## Unreleased +### Fixed + +- **A profile's sticky branch no longer follows `--url` to another server** + ([#171]). When `--url`/`NSC_URL` targets a different server from the + profile's, the profile `branch:` is ignored instead of failing with + `not_found` on the other server. `--branch` and `NSC_BRANCH` still apply. + ## v1.8.2 — 2026-10-23 Patch release. An invalid output format is now a usage error instead of a crash. @@ -121,6 +128,7 @@ 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 +[#171]: https://github.com/thomaschristory/netbox-super-cli/issues/171 [#179]: https://github.com/thomaschristory/netbox-super-cli/issues/179 [#173]: https://github.com/thomaschristory/netbox-super-cli/issues/173 [#150]: https://github.com/thomaschristory/netbox-super-cli/issues/150 diff --git a/docs/guides/branching.md b/docs/guides/branching.md index d402550..a3da3ba 100644 --- a/docs/guides/branching.md +++ b/docs/guides/branching.md @@ -34,6 +34,10 @@ 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. +A profile's sticky branch belongs to that profile's server. When `--url` or +`NSC_URL` points at a different server, it is ignored (`--branch` and +`NSC_BRANCH` still apply). + `--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 diff --git a/docs/guides/managing-profiles.md b/docs/guides/managing-profiles.md index dc56046..7565b94 100644 --- a/docs/guides/managing-profiles.md +++ b/docs/guides/managing-profiles.md @@ -110,7 +110,8 @@ Override precedence (highest first): CLI flag > env var > config profile. The same precedence applies to the NetBox Branching branch: `--branch NAME` > `NSC_BRANCH` > the profile's `branch:` key. `nsc branch use NAME` writes that key, and `nsc branch use --main` removes it. `--branch main` forces main for -one command. See [NetBox Branching](branching.md). +one command. The profile's `branch:` is ignored when `--url`/`NSC_URL` points +at a different server. See [NetBox Branching](branching.md). ```yaml profiles: diff --git a/nsc/cli/runtime.py b/nsc/cli/runtime.py index d8cc823..9c52833 100644 --- a/nsc/cli/runtime.py +++ b/nsc/cli/runtime.py @@ -13,7 +13,7 @@ from collections.abc import Iterable, Iterator, Mapping from typing import Any, Literal -from pydantic import BaseModel, ConfigDict, HttpUrl, SkipValidation +from pydantic import BaseModel, ConfigDict, HttpUrl, SkipValidation, ValidationError from nsc.auth.keyring_store import KeyringStoreError, resolve_token from nsc.branching.errors import ( @@ -171,8 +171,11 @@ def resolve_branch_selection( overrides: CLIOverrides, env: Mapping[str, str], ) -> BranchSelection | None: - name = overrides.profile or env.get("NSC_PROFILE") or config.default_profile - base = config.profiles.get(name) if name else None + base, _ = _select_base_profile(config, overrides, env) + url_override = _first_set(overrides.url, env.get("NSC_URL")) + if base is not None and url_override is not None and not _same_url(url_override, base.url): + # The sticky branch belongs to the profile's server, not to whatever --url targets. + base = None candidates: list[tuple[str | None, Literal["flag", "env", "profile"]]] = [ (overrides.branch, "flag"), (env.get("NSC_BRANCH"), "env"), @@ -195,6 +198,14 @@ def _select_base_profile( return config.profiles[name], name +def _same_url(raw: str, url: HttpUrl) -> bool: + try: + parsed = HttpUrl(raw) + except ValidationError: + return False + return str(parsed).rstrip("/") == str(url).rstrip("/") + + def _first_set(*values: str | None) -> str | None: for v in values: if v is not None and v != "": diff --git a/tests/cli/test_runtime_branch.py b/tests/cli/test_runtime_branch.py index 345090b..890bf49 100644 --- a/tests/cli/test_runtime_branch.py +++ b/tests/cli/test_runtime_branch.py @@ -155,3 +155,33 @@ def test_branch_before_subcommand_is_global_and_other_flags_still_scan() -> None 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" + + +@pytest.mark.parametrize( + ("overrides", "env"), + [ + (CLIOverrides(url="https://other.example"), {}), + (CLIOverrides(), {"NSC_URL": "https://other.example/"}), + ], +) +def test_profile_branch_ignored_when_url_targets_another_server( + overrides: CLIOverrides, env: dict[str, str] +) -> None: + assert resolve_branch_selection(_cfg(), overrides, env=env) is None + + +@pytest.mark.parametrize("url", ["https://nb.example", "https://nb.example/", "https://NB.example"]) +def test_profile_branch_kept_when_url_override_matches_profile(url: str) -> None: + sel = resolve_branch_selection(_cfg(), CLIOverrides(url=url), env={}) + assert sel == BranchSelection(name="prof-branch", source="profile") + + +def test_explicit_branch_still_applies_with_url_override() -> None: + other = CLIOverrides(url="https://other.example") + assert resolve_branch_selection(_cfg(), other, env={"NSC_BRANCH": "e"}) == BranchSelection( + name="e", source="env" + ) + with_flag = CLIOverrides(url="https://other.example", branch="f") + assert resolve_branch_selection(_cfg(), with_flag, env={}) == BranchSelection( + name="f", source="flag" + ) From eedd664fd335ff9f6c4a62109dab5dcfea634e42 Mon Sep 17 00:00:00 2001 From: Thomas Christory Date: Fri, 23 Oct 2026 16:12:08 +0200 Subject: [PATCH 2/2] fix(branching): refuse branch use on another server; notice when profile branch is ignored Review follow-ups for #171: nsc branch use / create --use wrote or cleared the profile's sticky branch while --url/NSC_URL targeted another server. They now fail with a config error before any write. When the profile branch is dropped because of a URL override, a one-line stderr notice says so. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++- docs/guides/branching.md | 6 +++-- nsc/cli/branch_commands.py | 35 ++++++++++++++++++-------- nsc/cli/globals.py | 7 ++++++ nsc/cli/runtime.py | 24 +++++++++++++----- tests/cli/test_branch_commands.py | 42 +++++++++++++++++++++++++++++-- tests/cli/test_runtime_branch.py | 31 ++++++++++++++++++++++- 7 files changed, 127 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97198a8..1da1932 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,10 @@ All notable changes to netbox-super-cli are tracked here. Format follows [Keep a - **A profile's sticky branch no longer follows `--url` to another server** ([#171]). When `--url`/`NSC_URL` targets a different server from the profile's, the profile `branch:` is ignored instead of failing with - `not_found` on the other server. `--branch` and `NSC_BRANCH` still apply. + `not_found` on the other server, and a one-line stderr notice says so. + `--branch` and `NSC_BRANCH` still apply. `nsc branch use` / `branch create + --use` now refuse (exit 12) to write or clear the profile's branch while a + URL override points at another server. ## v1.8.2 — 2026-10-23 diff --git a/docs/guides/branching.md b/docs/guides/branching.md index a3da3ba..24da642 100644 --- a/docs/guides/branching.md +++ b/docs/guides/branching.md @@ -35,8 +35,10 @@ The active branch comes from the first of these that is set: the profile has a sticky branch. A profile's sticky branch belongs to that profile's server. When `--url` or -`NSC_URL` points at a different server, it is ignored (`--branch` and -`NSC_BRANCH` still apply). +`NSC_URL` points at a different server, it is ignored, with a one-line notice +on stderr (`--branch` and `NSC_BRANCH` still apply). `nsc branch use` and +`nsc branch create --use` refuse to change the profile's branch while a URL +override points elsewhere. `--branch` is a global option and must come **before** the subcommand (`nsc --branch X dcim sites list`). After the subcommand, `--branch` belongs to diff --git a/nsc/cli/branch_commands.py b/nsc/cli/branch_commands.py index eb5b1a8..5e80bf9 100644 --- a/nsc/cli/branch_commands.py +++ b/nsc/cli/branch_commands.py @@ -30,7 +30,7 @@ from nsc.branching.models import Branch, choice_value from nsc.branching.resolve import get_branch, require_ready from nsc.cache.store import ADHOC_PROFILE -from nsc.cli.runtime import MAIN_BRANCH, RuntimeContext, emit_envelope, map_error +from nsc.cli.runtime import MAIN_BRANCH, RuntimeContext, emit_envelope, map_error, same_server from nsc.config.models import OutputFormat from nsc.http.client import NetBoxClient from nsc.http.errors import NetBoxAPIError, NetBoxClientError @@ -165,17 +165,30 @@ def _write_profile_branch(runtime: RuntimeContext, value: str | None) -> None: def _require_saved_profile(runtime: RuntimeContext, name: str | None) -> None: - if runtime.resolved_profile.name != ADHOC_PROFILE: - return + profile = runtime.resolved_profile hint = name or "" - _fail( - runtime, - ErrorEnvelope( - error="no saved profile to store the branch on; " - f"set NSC_BRANCH={hint} or pass --branch {hint}", - type=ErrorType.CONFIG, - ), - ) + if profile.name == ADHOC_PROFILE: + _fail( + runtime, + ErrorEnvelope( + error="no saved profile to store the branch on; " + f"set NSC_BRANCH={hint} or pass --branch {hint}", + type=ErrorType.CONFIG, + ), + ) + saved = runtime.config.profiles.get(profile.name) + if saved is not None and not same_server(str(profile.url), saved.url): + # A sticky branch is per profile server; storing one picked on another + # server would apply it to the wrong NetBox. + _fail( + runtime, + ErrorEnvelope( + error=f"--url/NSC_URL targets {profile.url}, not profile {profile.name}'s " + f"server {saved.url}; drop the URL override, or use --branch {hint} / " + f"NSC_BRANCH={hint} for this server", + type=ErrorType.CONFIG, + ), + ) def _use(runtime: RuntimeContext, branch: Branch) -> None: diff --git a/nsc/cli/globals.py b/nsc/cli/globals.py index 252b314..14900c8 100644 --- a/nsc/cli/globals.py +++ b/nsc/cli/globals.py @@ -11,6 +11,7 @@ from nsc.cli.runtime import ( CLIOverrides, RuntimeContext, + ignored_profile_branch, resolve_branch_selection, resolve_color, resolve_object_colors, @@ -48,6 +49,12 @@ def build_runtime_context(state: GlobalState) -> RuntimeContext: force_refresh=state.overrides.refresh_schema, ) branch = resolve_branch_selection(state.config, state.overrides, env=os.environ) + ignored = ignored_profile_branch(state.config, state.overrides, env=os.environ) + if ignored is not None and branch is None: + print( + f"⎇ profile branch {ignored} ignored: --url/NSC_URL targets another server", + file=sys.stderr, + ) client = NetBoxClient( profile, # type: ignore[arg-type] debug=state.debug, diff --git a/nsc/cli/runtime.py b/nsc/cli/runtime.py index 9c52833..bcc86e1 100644 --- a/nsc/cli/runtime.py +++ b/nsc/cli/runtime.py @@ -172,14 +172,13 @@ def resolve_branch_selection( env: Mapping[str, str], ) -> BranchSelection | None: base, _ = _select_base_profile(config, overrides, env) - url_override = _first_set(overrides.url, env.get("NSC_URL")) - if base is not None and url_override is not None and not _same_url(url_override, base.url): - # The sticky branch belongs to the profile's server, not to whatever --url targets. - base = None + profile_branch = base.branch if base else None + if ignored_profile_branch(config, overrides, env) is not None: + profile_branch = None candidates: list[tuple[str | None, Literal["flag", "env", "profile"]]] = [ (overrides.branch, "flag"), (env.get("NSC_BRANCH"), "env"), - (base.branch if base else None, "profile"), + (profile_branch, "profile"), ] for value, source in candidates: if value: @@ -187,6 +186,19 @@ def resolve_branch_selection( return None +def ignored_profile_branch( + config: Config, overrides: CLIOverrides, env: Mapping[str, str] +) -> str | None: + """The profile's sticky branch, when --url/NSC_URL points at another server.""" + base, _ = _select_base_profile(config, overrides, env) + if base is None or not base.branch: + return None + url_override = _first_set(overrides.url, env.get("NSC_URL")) + if url_override is None or same_server(url_override, base.url): + return None + return base.branch + + def _select_base_profile( config: Config, overrides: CLIOverrides, env: Mapping[str, str] ) -> tuple[Profile | None, str]: @@ -198,7 +210,7 @@ def _select_base_profile( return config.profiles[name], name -def _same_url(raw: str, url: HttpUrl) -> bool: +def same_server(raw: str, url: HttpUrl) -> bool: try: parsed = HttpUrl(raw) except ValidationError: diff --git a/tests/cli/test_branch_commands.py b/tests/cli/test_branch_commands.py index ff2558a..eb2ecb2 100644 --- a/tests/cli/test_branch_commands.py +++ b/tests/cli/test_branch_commands.py @@ -26,10 +26,10 @@ def _home(monkeypatch: pytest.MonkeyPatch, fixture_profile_yaml: Path) -> Path: return fixture_profile_yaml -def _mock_schema(router: respx.Router) -> None: +def _mock_schema(router: respx.Router, base: str = NB) -> None: bundled = next(Path("nsc/schemas/bundled").glob("*.json*")) body = gzip.decompress(bundled.read_bytes()) - router.get(f"{NB}/api/schema/?format=json").respond( + router.get(f"{base}/api/schema/?format=json").respond( 200, content=body, headers={"content-type": "application/json"} ) @@ -289,3 +289,41 @@ def test_saved_filter_branch_failure_honours_json_output( assert envelope["type"] == "config" assert envelope["details"]["branch"] == "my-change" assert not sites.called + + +OTHER = "https://other.example" + + +@pytest.mark.httpx2(assert_all_called=False) +@pytest.mark.parametrize("args", [["use", "--main"], ["use", "my-change"]]) +def test_use_refuses_when_url_targets_another_server( + httpx2_mock: respx.Router, _home: Path, args: list[str] +) -> None: + _mock_schema(httpx2_mock, OTHER) + _set_profile_branch(_home, "keep-me") + before = (_home / "config.yaml").read_text() + result = _invoke("--url", OTHER, "branch", *args, "--output", "json") + assert result.exit_code == 12, result.output + assert OTHER in json.loads(result.stdout)["error"] + assert (_home / "config.yaml").read_text() == before + + +@pytest.mark.httpx2(assert_all_called=False) +def test_create_use_refuses_before_creating_on_another_server( + httpx2_mock: respx.Router, _home: Path +) -> None: + _mock_schema(httpx2_mock, OTHER) + create = httpx2_mock.post(f"{OTHER}/api/plugins/branching/branches/").respond(201, json={}) + result = _invoke("--url", OTHER, "branch", "create", "x", "--use", "--output", "json") + assert result.exit_code == 12, result.output + assert not create.called + + +@pytest.mark.httpx2(assert_all_called=False) +def test_ignored_profile_branch_prints_notice(httpx2_mock: respx.Router, _home: Path) -> None: + _mock_schema(httpx2_mock, OTHER) + _set_profile_branch(_home, "keep-me") + result = _invoke("--url", OTHER, "branch", "current", "--output", "json") + assert result.exit_code == 0, result.output + assert "keep-me" in result.stderr + assert "ignored" in result.stderr diff --git a/tests/cli/test_runtime_branch.py b/tests/cli/test_runtime_branch.py index 890bf49..1827821 100644 --- a/tests/cli/test_runtime_branch.py +++ b/tests/cli/test_runtime_branch.py @@ -15,7 +15,13 @@ from nsc.cli import globals as globals_mod 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.cli.runtime import ( + BranchSelection, + CLIOverrides, + ignored_profile_branch, + map_error, + resolve_branch_selection, +) from nsc.config.loader import load_config from nsc.config.models import Config, Profile from nsc.model.command_model import CommandModel @@ -185,3 +191,26 @@ def test_explicit_branch_still_applies_with_url_override() -> None: assert resolve_branch_selection(_cfg(), with_flag, env={}) == BranchSelection( name="f", source="flag" ) + + +def _prefixed_cfg() -> Config: + return Config( + default_profile="p", + profiles={"p": Profile(name="p", url="https://nb.example/netbox", token="t", branch="pb")}, + ) + + +def test_profile_branch_with_path_prefix_matches_only_same_prefix() -> None: + same = CLIOverrides(url="https://nb.example/netbox/") + assert resolve_branch_selection(_prefixed_cfg(), same, env={}) == BranchSelection( + name="pb", source="profile" + ) + root = CLIOverrides(url="https://nb.example") + assert resolve_branch_selection(_prefixed_cfg(), root, env={}) is None + + +def test_ignored_profile_branch_is_reported() -> None: + other = CLIOverrides(url="https://other.example") + assert ignored_profile_branch(_cfg(), other, env={}) == "prof-branch" + assert ignored_profile_branch(_cfg(), CLIOverrides(), env={}) is None + assert ignored_profile_branch(_cfg(branch=None), other, env={}) is None