checker: reorder Status with negatives for good, JSON as string

Make StatusUnknown the zero value (0) so an uninitialized CheckState
reads as "no signal yet" rather than as healthy. Push StatusOK and
StatusInfo to negative values so the natural int ordering matches
severity ordering: aggregators can simply take max() to compute the
worst status, and Unknown correctly sits above OK/Info but below Warn.

Status now (un)marshals as its string name ("OK", "WARN", ...) so the
wire format is stable across any future renumbering. UnmarshalJSON
still accepts raw ints for backward compatibility with older snapshots
and clients.

checker/registry.go

@@ -27,8 +27,16 @@ var checkerRegistry = map[string]*CheckerDefinition{}
 // keyed by ObservationKey.
 var observationProviderRegistry = map[ObservationKey]ObservationProvider{}
 
-// RegisterChecker registers a checker definition globally.
+// RegisterChecker registers a checker definition globally. A second
+// registration under the same ID is refused with a warning rather than
+// silently overwriting the previous entry: in production this almost
+// always indicates a deployment mistake (two plugins shipping the same
+// checker, or a plugin shadowing a built-in).
 func RegisterChecker(c *CheckerDefinition) {
+	if _, exists := checkerRegistry[c.ID]; exists {
+		log.Printf("Warning: checker %q is already registered; ignoring duplicate registration", c.ID)
+		return
+	}
 	log.Println("Registering new checker:", c.ID)
 	c.BuildRulesInfo()
 	checkerRegistry[c.ID] = c
@@ -38,7 +46,16 @@ func RegisterChecker(c *CheckerDefinition) {
 // delegated to a remote HTTP endpoint. It appends an "endpoint" AdminOpt
 // so the administrator can optionally configure a remote URL.
 // When the endpoint is left empty, the checker runs locally as usual.
+//
+// The duplicate check happens before the AdminOpt append so that a
+// rejected second registration does not mutate the in-memory definition
+// of the already-registered checker (which a caller might still hold a
+// pointer to).
 func RegisterExternalizableChecker(c *CheckerDefinition) {
+	if _, exists := checkerRegistry[c.ID]; exists {
+		log.Printf("Warning: checker %q is already registered; ignoring duplicate registration", c.ID)
+		return
+	}
 	c.Options.AdminOpts = append(c.Options.AdminOpts,
 		CheckerOptionDocumentation{
 			Id:          "endpoint",
@@ -53,8 +70,15 @@ func RegisterExternalizableChecker(c *CheckerDefinition) {
 }
 
 // RegisterObservationProvider registers an observation provider globally.
+// A second registration under the same key is refused with a warning for
+// the same reason as RegisterChecker.
 func RegisterObservationProvider(p ObservationProvider) {
-	observationProviderRegistry[p.Key()] = p
+	key := p.Key()
+	if _, exists := observationProviderRegistry[key]; exists {
+		log.Printf("Warning: observation provider %q is already registered; ignoring duplicate registration", key)
+		return
+	}
+	observationProviderRegistry[key] = p
 }
 
 // GetCheckers returns all registered checker definitions.

checker/registry_test.go

@@ -0,0 +1,126 @@
+// Copyright 2020-2026 The happyDomain Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package checker
+
+import (
+	"context"
+	"testing"
+)
+
+// resetRegistries clears the global registries between tests so that one
+// test's registration cannot leak into the next. The package-level maps are
+// the only shared state.
+func resetRegistries() {
+	checkerRegistry = map[string]*CheckerDefinition{}
+	observationProviderRegistry = map[ObservationKey]ObservationProvider{}
+}
+
+type stubProvider struct {
+	key ObservationKey
+}
+
+func (s stubProvider) Key() ObservationKey { return s.key }
+func (s stubProvider) Collect(ctx context.Context, opts CheckerOptions) (any, error) {
+	return nil, nil
+}
+
+func TestRegisterChecker_DuplicateIgnored(t *testing.T) {
+	resetRegistries()
+
+	first := &CheckerDefinition{ID: "dup", Name: "First"}
+	second := &CheckerDefinition{ID: "dup", Name: "Second"}
+
+	RegisterChecker(first)
+	RegisterChecker(second)
+
+	got := FindChecker("dup")
+	if got == nil {
+		t.Fatal("expected checker to remain registered after duplicate")
+	}
+	if got.Name != "First" {
+		t.Errorf("duplicate registration overwrote original: got Name=%q, want %q", got.Name, "First")
+	}
+	if len(GetCheckers()) != 1 {
+		t.Errorf("registry has %d entries, want 1", len(GetCheckers()))
+	}
+}
+
+func TestRegisterExternalizableChecker_AppendsEndpointOnce(t *testing.T) {
+	resetRegistries()
+
+	c := &CheckerDefinition{ID: "ext", Name: "Ext"}
+
+	RegisterExternalizableChecker(c)
+	if n := len(c.Options.AdminOpts); n != 1 {
+		t.Fatalf("first registration: AdminOpts has %d entries, want 1", n)
+	}
+	if c.Options.AdminOpts[0].Id != "endpoint" {
+		t.Errorf("expected first AdminOpt id %q, got %q", "endpoint", c.Options.AdminOpts[0].Id)
+	}
+
+	// Second registration of the same definition pointer must NOT append a
+	// second "endpoint" AdminOpt — the duplicate check has to fire before
+	// the append, otherwise we silently mutate the live definition.
+	RegisterExternalizableChecker(c)
+	if n := len(c.Options.AdminOpts); n != 1 {
+		t.Errorf("after duplicate registration: AdminOpts has %d entries, want 1", n)
+	}
+}
+
+func TestRegisterExternalizableChecker_DuplicateDifferentPointerIgnored(t *testing.T) {
+	resetRegistries()
+
+	first := &CheckerDefinition{ID: "ext", Name: "First"}
+	second := &CheckerDefinition{ID: "ext", Name: "Second"}
+
+	RegisterExternalizableChecker(first)
+	RegisterExternalizableChecker(second)
+
+	got := FindChecker("ext")
+	if got == nil || got.Name != "First" {
+		t.Errorf("expected first registration to win, got %+v", got)
+	}
+	// The rejected second definition must not have been mutated either.
+	if len(second.Options.AdminOpts) != 0 {
+		t.Errorf("rejected definition was mutated: AdminOpts=%+v", second.Options.AdminOpts)
+	}
+}
+
+func TestRegisterObservationProvider_DuplicateIgnored(t *testing.T) {
+	resetRegistries()
+
+	first := stubProvider{key: "dns.A"}
+	second := stubProvider{key: "dns.A"}
+
+	RegisterObservationProvider(first)
+	RegisterObservationProvider(second)
+
+	got := FindObservationProvider("dns.A")
+	if got == nil {
+		t.Fatal("expected observation provider to remain registered after duplicate")
+	}
+	// Identity check: the registry must still hold the first instance.
+	if _, ok := got.(stubProvider); !ok {
+		t.Fatalf("unexpected provider type %T", got)
+	}
+	if got != ObservationProvider(first) {
+		// Both stubProvider values compare equal by value, so this also
+		// guards against an accidental overwrite-then-restore pattern.
+		t.Errorf("registry no longer holds the first registration")
+	}
+	if len(GetObservationProviders()) != 1 {
+		t.Errorf("registry has %d entries, want 1", len(GetObservationProviders()))
+	}
+}

checker/types.go

@@ -155,15 +155,21 @@ type CheckerOptionsDocumentation struct {
 }
 
 // Status represents the result status of a check evaluation.
+//
+// Numeric ordering is severity ordering: lower = better, higher = worse.
+// StatusUnknown is intentionally the zero value, so an uninitialized
+// CheckState reads as "no signal yet" rather than as a healthy OK.
+// "Good" statuses are negative so that aggregators can simply take the
+// max() of a set of statuses to compute the worst one.
 type Status int
 
 const (
-	StatusUnknown Status = iota
+	StatusOK      Status = -2
-	StatusOK
+	StatusInfo    Status = -1
-	StatusInfo
+	StatusUnknown Status = 0 // zero value: not initialized / no signal yet
-	StatusWarn
+	StatusWarn    Status = 1
-	StatusCrit
+	StatusCrit    Status = 2
-	StatusError
+	StatusError   Status = 3
 )
 
 // String returns the human-readable name of the status.
@@ -186,6 +192,46 @@ func (s Status) String() string {
 	}
 }
 
+// MarshalJSON serializes Status as its string name so the wire format
+// is stable across any future reordering of the underlying int values.
+func (s Status) MarshalJSON() ([]byte, error) {
+	return json.Marshal(s.String())
+}
+
+// UnmarshalJSON accepts either the string name (preferred) or a raw int
+// (for backward compatibility with older clients/snapshots).
+func (s *Status) UnmarshalJSON(data []byte) error {
+	if len(data) > 0 && data[0] == '"' {
+		var name string
+		if err := json.Unmarshal(data, &name); err != nil {
+			return err
+		}
+		switch name {
+		case "OK":
+			*s = StatusOK
+		case "INFO":
+			*s = StatusInfo
+		case "UNKNOWN", "":
+			*s = StatusUnknown
+		case "WARN":
+			*s = StatusWarn
+		case "CRIT":
+			*s = StatusCrit
+		case "ERROR":
+			*s = StatusError
+		default:
+			return fmt.Errorf("unknown status %q", name)
+		}
+		return nil
+	}
+	var n int
+	if err := json.Unmarshal(data, &n); err != nil {
+		return err
+	}
+	*s = Status(n)
+	return nil
+}
+
 // CheckState is the result of evaluating a single rule.
 type CheckState struct {
 	Status  Status         `json:"status"`