fix(group-tools): skip an unscoped blueprint instead of crashing on it

[neilmartin83]

pro group-tools analyze --unused panics with a nil pointer dereference on any tenant holding one blueprint that targets nothing. It dies after six sweeps of the instance have already been spent, and the traceback names an SDK type rather than the blueprint, so there is nothing an operator can do about it from the command line.

Found while verifying #387 against a second tenant. Pre-existing on main — reproduced on 64671e97 as well as on #387's branch, so it is neither #387's nor #389's.

Checking platform blueprints and benchmarks...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0]
  commands.addPlatformReferencedGroups(...) internal/commands/pro_group_tools.go:338
  commands.runGroupToolsAnalyzeUnused(...)  internal/commands/pro_group_tools.go:291

Cause

BlueprintDetail.Scope is *BlueprintScope with omitempty, so a blueprint with no targets arrives with it nil, and the loop dereferenced it unguarded.

The if err != nil { continue } directly above looks like it covers this and does not: GetBlueprint returns &result on success, so err is nil, detail is non-nil, and the missing thing is the pointer field.

BenchmarkV2.Target is *TargetV2 with omitempty and was dereferenced the same way. That half had not fired only because the tenant which exposed the blueprint case happened to have every benchmark targeted — an untargeted benchmark reaches it identically. Fixing one loop and leaving the other would move the same crash behind a slightly rarer tenant shape.

Fix

Skip the unscoped record and carry on. continue, not return, and that distinction is the point: a guard that returned would pass a crash test and silently stop marking the rest of the collection as referenced, so --unused would then offer a group that a blueprint really does scope as a removal candidate. That is a worse failure than the panic, because it reads as a successful answer.

Verification

TestAddPlatformReferencedGroups_ToleratesAnUnscopedBlueprintAndBenchmark serves an unscoped blueprint and an untargeted benchmark each followed by a scoped one, so it asserts both that the walk survives and that it keeps reading past the nil record. Mutation-checked: removing either guard reproduces the identical panic inside the test.

Live, against the gateway tenant that exposed it: the command completed and returned its rows, where before it produced no output and a stack trace.

go build ./..., go test ./..., make lint (0 issues) all clean.

Reproducing it, and why you probably cannot do it through the CLI

The gateway refuses to create this state on the current API version, so a reviewer who tries to manufacture it will hit a 400 and may conclude the crash is unreachable:

POST   [NotEmpty]     scope.deviceGroups: must not be empty
PATCH  [BAD_REQUEST]  scope.deviceGroups: must not be empty

Creating a blueprint with scope omitted is refused, and so is emptying an existing blueprint's scope by removing its only group. (Probed on a second tenant by the session that wrote #387.)

The state is nonetheless common. On the tenant where this crashed, 6 of 23 blueprints return scope: null — 26% of them. They arrived some other way: the UI, an API version that did not validate, or a scope group deleted out from under the blueprint. So this is not a theoretical nil; it is the normal condition of a quarter of that tenant's blueprints, and any one of them is enough to take the command down.

Two consequences for review:

• The fix is correct regardless of how the state arises. The field is declared nilable (blueprints/types.go:164 and :1268 in SDK v1.1.0, two separate structs; the benchmark side matches), and the dereference was unguarded. Whether the API should permit an unscoped blueprint is a different question from whether the CLI may crash on one.
• It cannot have an end-to-end regression test, because the API will not let a test create the state. That is why the test synthesises the nil response at the HTTP boundary rather than driving a real blueprint through the gateway.

Not included

Whether --unused should report unscoped blueprints at all is a separate question — right now they simply do not contribute references, which is correct for the reference set but means an operator gets no signal that a blueprint is scoped to nothing. That belongs in its own change if it is wanted.

🤖 Generated with Claude Code