From d56f2f2ebca50802ad1d978b1d517885e8d053f2 Mon Sep 17 00:00:00 2001 From: Thomas Christory Date: Sat, 24 Oct 2026 23:17:56 +0200 Subject: [PATCH 1/2] fix(builder): one resource per plugin endpoint instead of one per plugin (#202) NetBox tags every plugin endpoint `plugins`, so _resource_from_path took the plugin name as the resource and every endpoint of a plugin collapsed into one resource whose CRUD slots overwrote each other (`nsc plugins branching list` listed changes, `create` created branches; branch lists, branch-events and branchable-models were unreachable). Derive (plugin, resource) from /api/plugins/// paths; the generic `plugins` tag becomes the plugin's URL segment, a plugin's own tag is kept. Bump MODEL_FORMAT_VERSION so cached models rebuild. Also adds the live e2e assertion deferred from #180: `nsc branching changes list --branch ` filters without switching branch. Claude-Session: https://claude.ai/code/session_01RD8NgCVtp3yYFd6NSZpbUb --- CHANGELOG.md | 13 ++++ docs/guides/working-with-plugins.md | 18 ++++- nsc/builder/build.py | 33 ++++++-- nsc/model/command_model.py | 2 +- tests/builder/test_plugin_paths.py | 113 ++++++++++++++++++++++++++++ tests/e2e/test_branching.py | 29 +++++++ 6 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 tests/builder/test_plugin_paths.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc8476..6cac205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to netbox-super-cli are tracked here. Format follows [Keep a ## Unreleased +### Fixed + +- **Plugin endpoints get one command group per resource** ([#202]). NetBox tags + every plugin endpoint `plugins`, so all of a plugin's endpoints were merged + into one `nsc plugins ` resource whose verbs overwrote each other. For + example, `nsc plugins branching list` listed *changes* while `create` created + *branches*, and some endpoints were unreachable. Plugin commands now follow the + path. **Command paths change:** + `nsc plugins branching list` → `nsc branching changes list`, and + `nsc branching branches list|get|create|merge-create|…`, `nsc branching branch-events list`, etc. + Cached command models are rebuilt automatically. + ## v1.8.7 — 2026-10-24 Patch release. A literal `-o` used as another option's value is no longer read as the output flag. @@ -193,6 +205,7 @@ notices, and a fix for `nsc login` reporting the wrong user. [#189]: https://github.com/thomaschristory/netbox-super-cli/issues/189 [#192]: https://github.com/thomaschristory/netbox-super-cli/issues/192 [#196]: https://github.com/thomaschristory/netbox-super-cli/issues/196 +[#202]: https://github.com/thomaschristory/netbox-super-cli/issues/202 [#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 diff --git a/docs/guides/working-with-plugins.md b/docs/guides/working-with-plugins.md index 2028fb3..929f8d3 100644 --- a/docs/guides/working-with-plugins.md +++ b/docs/guides/working-with-plugins.md @@ -6,8 +6,19 @@ automatically — no per-plugin code in `nsc`**. ## How plugin endpoints surface -If a plugin registers `tags: [my_plugin]` and a path `/api/plugins/my-plugin/widgets/`, -you get: +Each `/api/plugins///` path becomes its own resource. NetBox +tags every plugin endpoint with the generic `plugins` tag; `nsc` then groups +them under the plugin's URL segment instead. For example, NetBox Branching's +`/api/plugins/branching/branches/` and `/api/plugins/branching/changes/` give: + +```sh +nsc branching branches list +nsc branching branches merge-create 3 --apply +nsc branching changes list --branch +``` + +A plugin that sets its own tag keeps it. With `tags: [my_plugin]` and a path +`/api/plugins/my-plugin/widgets/`, you get: ```sh nsc my_plugin widgets list @@ -15,6 +26,9 @@ nsc my_plugin widgets get 7 nsc my_plugin widgets create --field name=foo --apply ``` +A built-in command with the same name (for example `branch`) takes precedence +over a plugin group. + Verbs are derived from HTTP method + `operationId` exactly as for core endpoints. Custom action endpoints (`POST /api/plugins/my-plugin/widgets/{id}/calibrate/`) become e.g. `nsc my_plugin widgets calibrate 7 --apply`. diff --git a/nsc/builder/build.py b/nsc/builder/build.py index 2c27316..4174249 100644 --- a/nsc/builder/build.py +++ b/nsc/builder/build.py @@ -52,6 +52,7 @@ ) # api / / = at least 3 non-empty path parts _MIN_PATH_PARTS = 3 +_GENERIC_PLUGIN_TAG = "plugins" def build_command_model(loaded: LoadedSchema) -> CommandModel: @@ -118,7 +119,16 @@ def _assimilate( if tag_name is None: return # untagged operations are skipped intentionally - resource_name, _ = _resource_from_path(path, tag_name) + plugin_route = _plugin_route(path) + if plugin_route is not None: + plugin_name, resource_name = plugin_route + # Issue #202: NetBox tags every plugin endpoint with the generic + # `plugins`; grouping on it merged all of a plugin's endpoints into one + # resource whose CRUD slots overwrote each other. + if tag_name == _GENERIC_PLUGIN_TAG: + tag_name = plugin_name + else: + resource_name, _ = _resource_from_path(path, tag_name) if resource_name is None: return # paths that don't match the /api///... shape @@ -136,6 +146,19 @@ def _assimilate( _attach(resource, _classify(http_method, path), op) +def _plugin_route(path: str) -> tuple[str, str | None] | None: + """`(plugin, resource)` for `/api/plugins///...`, else None. + + `resource` is None for the plugin's root view (`/api/plugins//`). + """ + parts = [p for p in path.split("/") if p] + if len(parts) < _MIN_PATH_PARTS or parts[:2] != ["api", _GENERIC_PLUGIN_TAG]: + return None + if len(parts) == _MIN_PATH_PARTS or _PARAM_SEGMENT.match(parts[3]): + return parts[2], None + return parts[2], parts[3] + + def _resource_from_path(path: str, tag: str) -> tuple[str | None, bool]: """Return (resource_name, is_collection_path). @@ -153,12 +176,8 @@ def _resource_from_path(path: str, tag: str) -> tuple[str | None, bool]: # Expect: api / / / [...] if len(parts) < _MIN_PATH_PARTS: return None, False - # Some plugin endpoints have nested tags (e.g., ['plugins', '']). - # The tag string from the spec is authoritative; just find . - # Strategy: skip leading non-parameter segments until we've consumed the - # tag's words and reached a resource segment. - # Easier heuristic that works for NetBox: the resource is the first - # segment after the segment that begins the tag's path-form. + # Plugin paths are handled by `_plugin_route`. Otherwise the resource is + # the first segment after the segment that begins the tag's path-form. tag_form = tag.replace(" ", "-").lower() try: anchor = parts.index(tag_form) diff --git a/nsc/model/command_model.py b/nsc/model/command_model.py index 284b1f0..8c12d02 100644 --- a/nsc/model/command_model.py +++ b/nsc/model/command_model.py @@ -46,7 +46,7 @@ class PrimitiveType(StrEnum): # Bumped whenever the builder changes the shape of a CommandModel in a way that # makes older cached models incomplete (not just a schema-body change). The cache # rebuilds entries stamped with an older version. See nsc/cache/store.py. -MODEL_FORMAT_VERSION = 1 +MODEL_FORMAT_VERSION = 2 class _Frozen(BaseModel): diff --git a/tests/builder/test_plugin_paths.py b/tests/builder/test_plugin_paths.py new file mode 100644 index 0000000..6ebf244 --- /dev/null +++ b/tests/builder/test_plugin_paths.py @@ -0,0 +1,113 @@ +"""Plugin endpoints get one resource per `/api/plugins///` path (#202).""" + +from __future__ import annotations + +import json +from typing import Any + +from nsc.builder.build import build_command_model +from nsc.model.command_model import CommandModel +from nsc.schema.hashing import canonical_sha256 +from nsc.schema.loader import LoadedSchema +from nsc.schema.models import OpenAPIDocument + + +def _op(operation_id: str, tag: str) -> dict[str, Any]: + return {"operationId": operation_id, "tags": [tag], "responses": {"200": {"description": "OK"}}} + + +def _build(paths: dict[str, dict[str, Any]]) -> CommandModel: + doc_dict: dict[str, Any] = { + "openapi": "3.0.3", + "info": {"title": "Test", "version": "0.0.1"}, + "paths": paths, + } + body = json.dumps(doc_dict).encode() + doc = OpenAPIDocument.model_validate(doc_dict) + loaded = LoadedSchema(source="", body=body, hash=canonical_sha256(body), document=doc) + return build_command_model(loaded) + + +def _branching_paths(tag: str = "plugins") -> dict[str, dict[str, Any]]: + # Shape of netbox-branching 1.2 on NetBox 4.7: every path is tagged `plugins`. + base = "/api/plugins/branching" + return { + f"{base}/branches/": { + "get": _op("plugins_branching_branches_list", tag), + "post": _op("plugins_branching_branches_create", tag), + }, + f"{base}/branches/{{id}}/": { + "get": _op("plugins_branching_branches_retrieve", tag), + "patch": _op("plugins_branching_branches_partial_update", tag), + "delete": _op("plugins_branching_branches_destroy", tag), + }, + f"{base}/branches/{{id}}/merge/": { + "post": _op("plugins_branching_branches_merge_create", tag) + }, + f"{base}/changes/": {"get": _op("plugins_branching_changes_list", tag)}, + f"{base}/changes/{{id}}/": {"get": _op("plugins_branching_changes_retrieve", tag)}, + f"{base}/branch-events/": {"get": _op("plugins_branching_branch_events_list", tag)}, + f"{base}/branchable-models/": {"get": _op("plugins_branching_branchable_models_list", tag)}, + } + + +def test_generic_plugins_tag_becomes_the_plugin_name() -> None: + model = _build(_branching_paths()) + assert "plugins" not in model.tags + assert set(model.tags["branching"].resources) == { + "branches", + "changes", + "branch-events", + "branchable-models", + } + + +def test_plugin_resources_keep_their_own_operations() -> None: + resources = _build(_branching_paths()).tags["branching"].resources + branches = resources["branches"] + assert branches.list_op is not None + assert branches.list_op.path == "/api/plugins/branching/branches/" + assert branches.create_op is not None + assert branches.create_op.path == "/api/plugins/branching/branches/" + assert branches.get_op is not None + assert branches.get_op.path == "/api/plugins/branching/branches/{id}/" + assert [a.path for a in branches.custom_actions] == [ + "/api/plugins/branching/branches/{id}/merge/" + ] + changes = resources["changes"] + assert changes.list_op is not None + assert changes.list_op.path == "/api/plugins/branching/changes/" + assert changes.create_op is None + + +def test_plugin_with_its_own_tag_keeps_it() -> None: + model = _build( + { + "/api/plugins/my-plugin/widgets/": { + "get": _op("plugins_my_plugin_widgets_list", "my_plugin") + } + } + ) + widgets = model.tags["my_plugin"].resources["widgets"] + assert widgets.list_op is not None + assert widgets.list_op.path == "/api/plugins/my-plugin/widgets/" + + +def test_two_plugins_under_the_generic_tag_stay_separate() -> None: + model = _build( + { + "/api/plugins/branching/changes/": { + "get": _op("plugins_branching_changes_list", "plugins") + }, + "/api/plugins/netbox-dns/zones/": { + "get": _op("plugins_netbox_dns_zones_list", "plugins") + }, + } + ) + assert set(model.tags["branching"].resources) == {"changes"} + assert set(model.tags["netbox-dns"].resources) == {"zones"} + + +def test_plugin_root_view_is_skipped() -> None: + model = _build({"/api/plugins/branching/": {"get": _op("plugins_branching_root", "plugins")}}) + assert model.tags == {} diff --git a/tests/e2e/test_branching.py b/tests/e2e/test_branching.py index 5f6d644..6c3472b 100644 --- a/tests/e2e/test_branching.py +++ b/tests/e2e/test_branching.py @@ -144,3 +144,32 @@ def test_conflict_on_merge(run_nsc: RunNsc, netbox_client: httpx2.Client, branch assert r.returncode == 0, r.stderr detail = netbox_client.get(f"/api/extras/tags/{tag_id}/").json() assert detail["description"] == "from-branch" + + +@pytest.mark.usefixtures("clean_tags") +def test_plugin_endpoints_and_subcommand_branch_filter(run_nsc: RunNsc, branch: str) -> None: + """#202: each plugin endpoint is its own resource. #170/#180: a subcommand's + `--branch` query filter does not switch the invocation into a branch.""" + tag = _name("tag") + r = run_nsc( + "extras", + "tags", + "create", + "--field", + f"name={tag}", + "--field", + f"slug={tag}", + "--apply", + env={"NSC_BRANCH": branch}, + ) + assert r.returncode == 0, r.stderr + + r = run_nsc("branching", "branches", "list", "--output", "json") + assert r.returncode == 0, r.stderr + branches = {b["name"]: b["schema_id"] for b in r.stdout_json()} # type: ignore[union-attr] + assert branch in branches + + r = run_nsc("branching", "changes", "list", "--branch", branches[branch], "--output", "json") + assert r.returncode == 0, r.stderr + assert "⎇" not in r.stderr + assert [c["object_repr"] for c in r.stdout_json()] == [tag] # type: ignore[union-attr] From 128a46a6e864be026fab33e76a5f837af8a9328b Mon Sep 17 00:00:00 2001 From: Thomas Christory Date: Sat, 24 Oct 2026 23:24:43 +0200 Subject: [PATCH 2/2] fix(builder): index plugin list serializers to their per-plugin resource _build_fk_resources parsed paths without the plugin route, so plugin serializers never mapped to a resource and TUI FK pickers could not resolve plugin targets. Share one `_route` helper between _assimilate and the FK index. Claude-Session: https://claude.ai/code/session_01RD8NgCVtp3yYFd6NSZpbUb --- nsc/builder/build.py | 34 ++++++++++++-------------- tests/builder/test_plugin_paths.py | 39 +++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/nsc/builder/build.py b/nsc/builder/build.py index 4174249..d63d05e 100644 --- a/nsc/builder/build.py +++ b/nsc/builder/build.py @@ -119,16 +119,7 @@ def _assimilate( if tag_name is None: return # untagged operations are skipped intentionally - plugin_route = _plugin_route(path) - if plugin_route is not None: - plugin_name, resource_name = plugin_route - # Issue #202: NetBox tags every plugin endpoint with the generic - # `plugins`; grouping on it merged all of a plugin's endpoints into one - # resource whose CRUD slots overwrote each other. - if tag_name == _GENERIC_PLUGIN_TAG: - tag_name = plugin_name - else: - resource_name, _ = _resource_from_path(path, tag_name) + tag_name, resource_name, _ = _route(path, tag_name) if resource_name is None: return # paths that don't match the /api///... shape @@ -146,17 +137,23 @@ def _assimilate( _attach(resource, _classify(http_method, path), op) -def _plugin_route(path: str) -> tuple[str, str | None] | None: - """`(plugin, resource)` for `/api/plugins///...`, else None. +def _route(path: str, spec_tag: str) -> tuple[str, str | None, bool]: + """Return (tag, resource_name, is_collection_path) for an operation's path. - `resource` is None for the plugin's root view (`/api/plugins//`). + Issue #202: NetBox tags every plugin endpoint with the generic `plugins`; + grouping on it merged all of a plugin's endpoints into one resource whose + CRUD slots overwrote each other. For `/api/plugins///...` + the resource comes from the path, and the generic tag becomes ``. """ parts = [p for p in path.split("/") if p] if len(parts) < _MIN_PATH_PARTS or parts[:2] != ["api", _GENERIC_PLUGIN_TAG]: - return None + resource, is_collection = _resource_from_path(path, spec_tag) + return spec_tag, resource, is_collection + tag = parts[2] if spec_tag == _GENERIC_PLUGIN_TAG else spec_tag + # `/api/plugins//` is the plugin's root view, not a resource. if len(parts) == _MIN_PATH_PARTS or _PARAM_SEGMENT.match(parts[3]): - return parts[2], None - return parts[2], parts[3] + return tag, None, False + return tag, parts[3], len(parts) == _MIN_PATH_PARTS + 1 def _resource_from_path(path: str, tag: str) -> tuple[str | None, bool]: @@ -176,7 +173,7 @@ def _resource_from_path(path: str, tag: str) -> tuple[str | None, bool]: # Expect: api / / / [...] if len(parts) < _MIN_PATH_PARTS: return None, False - # Plugin paths are handled by `_plugin_route`. Otherwise the resource is + # Plugin paths are handled by `_route`. Otherwise the resource is # the first segment after the segment that begins the tag's path-form. tag_form = tag.replace(" ", "-").lower() try: @@ -457,8 +454,7 @@ def _build_fk_resources(doc: OpenAPIDocument) -> dict[str, FkResourceRef]: get_op = doc.paths[path].get if get_op is None or not get_op.tags: continue - tag_name = get_op.tags[0] - resource_name, is_collection = _resource_from_path(path, tag_name) + tag_name, resource_name, is_collection = _route(path, get_op.tags[0]) if resource_name is None or not is_collection: continue serializer = _list_serializer_name(get_op, doc) diff --git a/tests/builder/test_plugin_paths.py b/tests/builder/test_plugin_paths.py index 6ebf244..9e5e45e 100644 --- a/tests/builder/test_plugin_paths.py +++ b/tests/builder/test_plugin_paths.py @@ -6,7 +6,7 @@ from typing import Any from nsc.builder.build import build_command_model -from nsc.model.command_model import CommandModel +from nsc.model.command_model import CommandModel, FkResourceRef from nsc.schema.hashing import canonical_sha256 from nsc.schema.loader import LoadedSchema from nsc.schema.models import OpenAPIDocument @@ -111,3 +111,40 @@ def test_two_plugins_under_the_generic_tag_stay_separate() -> None: def test_plugin_root_view_is_skipped() -> None: model = _build({"/api/plugins/branching/": {"get": _op("plugins_branching_root", "plugins")}}) assert model.tags == {} + + +def test_plugin_list_serializer_is_indexed_to_its_resource() -> None: + list_op = _op("plugins_branching_branches_list", "plugins") + list_op["responses"] = { + "200": { + "description": "OK", + "content": { + "application/json": {"schema": {"$ref": "#/components/schemas/PaginatedBranchList"}} + }, + } + } + doc_dict: dict[str, Any] = { + "openapi": "3.0.3", + "info": {"title": "Test", "version": "0.0.1"}, + "paths": {"/api/plugins/branching/branches/": {"get": list_op}}, + "components": { + "schemas": { + "PaginatedBranchList": { + "type": "object", + "properties": { + "results": { + "type": "array", + "items": {"$ref": "#/components/schemas/Branch"}, + } + }, + }, + "Branch": {"type": "object", "properties": {"id": {"type": "integer"}}}, + } + }, + } + body = json.dumps(doc_dict).encode() + doc = OpenAPIDocument.model_validate(doc_dict) + model = build_command_model( + LoadedSchema(source="", body=body, hash=canonical_sha256(body), document=doc) + ) + assert model.fk_resources["Branch"] == FkResourceRef(tag="branching", resource="branches")