refactor(managementapi): derive dependency guards from the Deps struct

Summary

Every Deps field (74 today) had a hand-written nil check in internal/managementapi and a duplicate in cmd/artifact-registry on the same construction path. S17 Phase 6 grew Deps from 26 to 73 fields, and 16 of its 41 MRs edited both files (73% co-change across 43 shared revisions). This MR derives the guard from the struct once: a reflective walk in NewHandler produces byte-identical panic messages, so a new store needs zero guard edits.

The non-obvious calls:

  • Scope goes past the issue's literal step 1: it deletes wireManagementAPIWithDeps's six inline Deps checks along with the six helper functions. The handler-side guard is a strict superset on the same path, and the wire layer keeps only its own argument guards.
  • The walk is the repo's first production field-enumeration reflection, named here as the accepted smell. The rejected alternative, a name-and-value table, reintroduces the per-field edit this refactor exists to remove.
  • The walk fails closed three ways: an unsupported field kind, a struct seam with zero checkable leaves, and an embedded unexported field all panic at boot rather than pass unguarded.
  • The exhaustive test enumerates the same leaves recursively, pins the count (80) as a new-field tripwire, and carries two spelled-out literal anchors, so a path-derivation bug shared by walk and test cannot agree past them.

Diff size, past the 500-LOC guideline: +296/-889 across 7 files. The deletion is atomic, because removing the wire-side guards is only safe in the same change that proves the handler-side guard exhaustive. 85% of the deleted lines are the mechanical removal of identical guard boilerplate (wire_management.go -382, handler.go -374). The additions are the walk, its white-box tests (requiredeps_internal_test.go +144), and the reflective enumeration in the panic test (handler_test.go +71).

Governing ADRs

None. The nearest candidate is ADR-023 (code structure and enforcement), which constrains imports across package boundaries. This change stays inside internal/managementapi and its existing wiring.

Testing

Behavior Pinned by
All 80 leaf panic messages byte-identical to the old flat guards (characterization: green against them before the rewrite, recorded in the test commit body) TestNewHandler_PanicsOnNilDependency
Dotted-path struct recursion the 9 ConnectionTestSource member subtests, plus the white-box TestRequireNonNilSeams_NamesNestedSeamsByDottedPath
Fail-closed arms (unsupported kind, zero-leaf struct seam, embedded unexported field) TestRequireNonNilSeams_RefusesAnUnsupportedKind, _RefusesAStructSeamWithNoCheckableLeaf, _RefusesAnEmbeddedUnexportedField
Leaf-count pin, 80 require.Len in TestNewHandler_PanicsOnNilDependency
The wire layer keeps no Deps guards, so a nil seam's escaping panic is exactly NewHandler's TestWireManagementAPIWithDeps_NilSeamPanicsCarryNewHandlerMessage
wireManagementAPI argument guards untouched TestWireManagementAPI_PanicsOnNilDependencies

No e2e scenario is added or affected: behavior-preserving refactor with no route, store, or wiring changes. The only observable difference is the panic-message prefix on mis-wiring paths, which only tests observe.

Closes #874 (closed)

Context for LLM agents

Rationale

  • Name-and-value guard table. Rejected: every new field needs a table row, which is the per-field edit this refactor removes.
  • Keeping the wire-side inline Deps checks (the issue's literal step 1 deletes only the six helpers). Rejected: NewHandler's walk checks a strict superset on the same construction path, so the inline checks add no coverage and keep the duplicate-edit cost.
  • Chosen: reflective walk in NewHandler, byte-identical messages, fail-closed on any field shape it cannot check.

Consequences: with multiple nil fields, the first-firing check now follows declaration order (it diverged in three places before). Safe because no test builds a partial Deps relying on which check fires first.

Non-goals

  • pact_provider_test.go's buildPactHandler sets 17 of 74 fields and panics whenever it actually runs (dormant, unwired in CI, pre-existing). Its trigger and message are unchanged by this MR. Follow-up candidate.
  • NewBulkDeleteNpmWorker's missing guard table. BulkDeleteNpmDeps has no nil checks today, a separate gap with the same candidate fix.
  • Restructuring Deps into sub-structs, and CredentialManager guard coverage. Adjacent designs, deliberately excluded here.

Merge request reports

Loading
Loading