fix(config,app)!: a delivered-empty numeric config value fails startup

CLAUDE.md

@@ -423,6 +423,7 @@ GoBricks breaks its own API surface when justified. Greenfield work uses the new
 - **keystore.secretminlength tri-state (ADR-065):** `KeyStoreConfig.SecretMinLength` is `*int` (`new(n)` in Go literals; nil = 32, `0` = off, deprecated); a hand-built config that left it unset now enforces the 32-byte floor.
 - **Dead app lifecycle surface removed (ADR-067):** `MessagingInitializer` and `ConnectionPreWarmer` (constructors and methods included), `Options.Database` and `Options.MessagingClient` are gone; the eight debug response types are unexported with their JSON unchanged.
 - **One delivery pipeline (ADR-068):** `messaging.StartConsumeSpan` is removed — a service driving its own consume loop starts its own span — and the AMQP `messaging.client.consumed.messages` counter is recorded at completion with `error.type` instead of at receive without it.
+- **Delivered-empty numeric config (ADR-074):** an empty string bound to a numeric key (`FOO=`, empty `secretKeyRef`) fails startup naming the key instead of decoding as `0`; `FOO=0`, unset, YAML omission and YAML null are unchanged.
 - **Scheduler timeouts normalize (ADR-075):** `scheduler.timeout.shutdown`/`slowjob` default in `config.Validate` (30s/25s — hand-built configs move 30s → 25s), negatives fail validation, and `scheduler.Module.Init` requires a NORMALIZED `deps.Config` — non-nil, both timeouts positive — so a config assembled outside app construction must go through `config.Validate` first.
 - **Section-qualified config errors (ADR-076):** a non-root database section's `ConfigError.Field` names that section (`databases.reporting.host`, `multitenant.tenants.acme.database.host`); match with a database-scoped predicate, not equality and not a bare suffix — `cache.redis.host` ends in `.host` too. Root and the connect door keep the root spelling.
 

app/bootstrap.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 
 	"github.com/gaborage/go-bricks/config"
@@ -130,7 +131,18 @@ func (b *appBootstrap) dependencies(startupCtx context.Context) (*dependencyBund
 	}
 
 	// Initialize observability provider (no-op if disabled)
-	obsProvider := b.initializeObservability(startupCtx)
+	obsProvider, err := b.initializeObservability(startupCtx)
+	if err != nil {
+		if cacheManager != nil {
+			_ = cacheManager.Close()
+		}
+		closeManagers := b.closeManagers
+		if closeManagers == nil {
+			closeManagers = closeManagersOnDependencyError
+		}
+		closeManagers(dbManager, messagingManager)
+		return nil, err
+	}
 
 	// Enhance logger with OTLP export if enabled
 	// This upgrades the bootstrap logger so all subsequent components share a single
@@ -213,19 +225,34 @@ func (b *appBootstrap) warnIfDatabaseAbsent() {
 		"if this service expects a database, its configuration did not reach the process")
 }
 
+// observabilityConfigKey is the koanf section initializeObservability decodes; its presence
+// separates "no observability configured" from "configured, but undecodable".
+const observabilityConfigKey = "observability"
+
 // initializeObservability creates and configures the observability provider.
-// Returns a no-op provider if observability is disabled or configuration is missing.
-func (b *appBootstrap) initializeObservability(startupCtx context.Context) observability.Provider {
+// Returns a no-op provider when the observability section is absent.
+//
+// A section that IS present but cannot be decoded aborts startup instead: degrading to
+// the no-op provider there turns one bad key into total telemetry loss — no traces, no
+// metrics, no OTLP logs, no migration audit events — announced by a single WARN on the
+// way past. That is exactly the shape a delivered-empty numeric now produces (ADR-074),
+// so the distinction has to be drawn rather than assumed.
+func (b *appBootstrap) initializeObservability(startupCtx context.Context) (observability.Provider, error) {
 	b.log.Debug().Msg("Starting observability initialization")
 
 	// Create observability config
 	var obsCfg observability.Config
 
-	// Try to unmarshal configuration from the "observability" key
-	if err := b.cfg.Unmarshal("observability", &obsCfg); err != nil {
-		// Configuration missing or invalid - use defaults (observability disabled)
-		b.log.Warn().Err(err).Msg("Observability configuration not found or invalid, using no-op provider")
-		return observability.MustNewProvider(&observability.Config{Enabled: false})
+	// Absence is decided BEFORE decoding, not inferred from a decode error: koanf returns no
+	// error for a missing key, so an absent section would otherwise decode to a zero Config
+	// and fall through to construction, leaving the documented no-op branch unreachable.
+	if !b.cfg.Exists(observabilityConfigKey) {
+		b.log.Debug().Msg("No observability configuration, using no-op provider")
+		return observability.MustNewProvider(&observability.Config{Enabled: false}), nil
+	}
+
+	if err := b.cfg.Unmarshal(observabilityConfigKey, &obsCfg); err != nil {
+		return nil, fmt.Errorf("observability configuration is present but invalid: %w", err)
 	}
 
 	b.log.Debug().
@@ -287,8 +314,10 @@ func (b *appBootstrap) initializeObservability(startupCtx context.Context) obser
 
 	provider, err := construct(ctx, &obsCfg)
 	if err != nil {
+		// Construction failure is an environment problem (unreachable collector, bad
+		// resource probe), not a malformed config: it stays non-fatal, as before.
 		b.log.Warn().Err(err).Msg("Failed to initialize observability, using no-op provider")
-		return observability.MustNewProvider(&observability.Config{Enabled: false})
+		return observability.MustNewProvider(&observability.Config{Enabled: false}), nil
 	}
 
 	if obsCfg.Enabled {
@@ -303,7 +332,7 @@ func (b *appBootstrap) initializeObservability(startupCtx context.Context) obser
 		b.log.Debug().Msg("Observability disabled by configuration")
 	}
 
-	return provider
+	return provider, nil
 }
 
 // enhanceLoggerWithOTel attaches OTLP log export to the logger if observability is enabled.

app/bootstrap_test.go

@@ -310,7 +310,8 @@ observability:
 	bootstrap := newAppBootstrap(cfg, log, &Options{})
 
 	// Initialize observability
-	obsProvider := bootstrap.initializeObservability(context.Background())
+	obsProvider, err := bootstrap.initializeObservability(context.Background())
+	require.NoError(t, err)
 	require.NotNil(t, obsProvider)
 
 	// Verify tracer provider is initialized
@@ -415,7 +416,8 @@ observability:
 	bootstrap := newAppBootstrap(cfg, log, &Options{})
 
 	// Initialize observability
-	obsProvider := bootstrap.initializeObservability(context.Background())
+	obsProvider, err := bootstrap.initializeObservability(context.Background())
+	require.NoError(t, err)
 	require.NotNil(t, obsProvider)
 
 	// Verify provider is functional (indicates config was loaded successfully)
@@ -497,7 +499,8 @@ observability:
 	bootstrap := newAppBootstrap(cfg, log, &Options{})
 
 	// Initialize observability
-	obsProvider := bootstrap.initializeObservability(context.Background())
+	obsProvider, err := bootstrap.initializeObservability(context.Background())
+	require.NoError(t, err)
 	require.NotNil(t, obsProvider)
 
 	// Should return noop providers
@@ -556,7 +559,8 @@ debug:
 	bootstrap := newAppBootstrap(cfg, log, &Options{})
 
 	// Initialize observability (should fallback to noop provider)
-	obsProvider := bootstrap.initializeObservability(context.Background())
+	obsProvider, err := bootstrap.initializeObservability(context.Background())
+	require.NoError(t, err)
 	require.NotNil(t, obsProvider, "Should return noop provider when config is missing")
 
 	// Should return noop providers
@@ -571,6 +575,77 @@ debug:
 	assert.NoError(t, err)
 }
 
+// TestInitializeObservabilityFailsClosedOnUndecodableSection pins the distinction the
+// no-op fallback used to blur. observability.* is decoded separately from config.Config,
+// so a delivered-empty numeric there passes config.Load and lands here (ADR-074) — and
+// swallowing it took every trace, metric, OTLP log and migration audit event with it,
+// announced by one WARN. A present-but-undecodable section now aborts startup; an ABSENT
+// section keeps the documented no-op posture.
+func TestInitializeObservabilityFailsClosedOnUndecodableSection(t *testing.T) {
+	t.Run("present_but_undecodable_aborts", func(t *testing.T) {
+		cfg := loadConfigFromYAML(t, `
+app:
+  name: "test"
+  version: "1.0.0"
+server:
+  port: 8080
+observability:
+  enabled: true
+  trace:
+    batch:
+      size: ""
+`)
+		bootstrap := newAppBootstrap(cfg, logger.New("info", false), &Options{})
+
+		provider, err := bootstrap.initializeObservability(context.Background())
+
+		require.Error(t, err)
+		assert.ErrorContains(t, err, "present but invalid")
+		assert.ErrorContains(t, err, "delivered empty")
+		assert.Nil(t, provider, "a caller must not receive a silently degraded provider")
+	})
+
+	t.Run("absent_section_never_reaches_construction", func(t *testing.T) {
+		cfg := loadConfigFromYAML(t, `
+app:
+  name: "test"
+  version: "1.0.0"
+server:
+  port: 8080
+`)
+		bootstrap := newAppBootstrap(cfg, logger.New("info", false), &Options{})
+		constructed := false
+		bootstrap.newProvider = func(context.Context, *observability.Config) (observability.Provider, error) {
+			constructed = true
+			return observability.MustNewProvider(&observability.Config{Enabled: false}), nil
+		}
+
+		provider, err := bootstrap.initializeObservability(context.Background())
+
+		require.NoError(t, err)
+		require.NotNil(t, provider)
+		assert.False(t, constructed,
+			"an absent section takes the documented no-op branch; koanf returns no error for a "+
+				"missing key, so inferring absence from a decode failure never fires")
+	})
+
+	t.Run("absent_section_stays_no_op", func(t *testing.T) {
+		cfg := loadConfigFromYAML(t, `
+app:
+  name: "test"
+  version: "1.0.0"
+server:
+  port: 8080
+`)
+		bootstrap := newAppBootstrap(cfg, logger.New("info", false), &Options{})
+
+		provider, err := bootstrap.initializeObservability(context.Background())
+
+		require.NoError(t, err)
+		require.NotNil(t, provider)
+	})
+}
+
 // TestInitializeObservabilityThreadsBudgetContext verifies that when
 // app.startup.observability is positive, the construction seam receives a
 // non-nil context whose deadline is set roughly the budget into the future.
@@ -619,7 +694,8 @@ observability:
 	}
 
 	start := time.Now()
-	got := bootstrap.initializeObservability(context.Background())
+	got, err := bootstrap.initializeObservability(context.Background())
+	require.NoError(t, err)
 
 	assert.Same(t, want, got, "the provider returned by the seam should be installed verbatim")
 	require.NotNil(t, gotCtx, "seam must receive a non-nil context")
@@ -672,7 +748,8 @@ observability:
 	parent, cancel := context.WithCancel(context.Background())
 	cancel() // cancel the parent BEFORE construction
 
-	bootstrap.initializeObservability(parent)
+	_, err := bootstrap.initializeObservability(parent)
+	require.NoError(t, err)
 
 	require.NotNil(t, gotCtx, "seam must receive a non-nil context")
 	require.ErrorIs(t, gotCtx.Err(), context.Canceled,
@@ -715,7 +792,8 @@ observability:
 		return nil, errors.New("exporter dial failed")
 	}
 
-	got := bootstrap.initializeObservability(context.Background())
+	got, err := bootstrap.initializeObservability(context.Background())
+	require.NoError(t, err)
 
 	require.NotNil(t, got, "a no-op provider must be installed on constructor error")
 	assert.NotNil(t, got.TracerProvider(), "no-op provider must expose a tracer provider")

config/config.go

@@ -281,12 +281,14 @@ func tryLoadYAMLFile(k *koanf.Koanf, baseName string) error {
 }
 
 // buildDecoderConfig is the decoder for Load: it replicates koanf's default Unmarshal
-// decoder (knadh/koanf/v2 koanf.go:265-272) plus the numeric-duration guard and the
-// comma-split slice hook (so a single env var can express a []string). koanf fills in
+// decoder (knadh/koanf/v2 koanf.go:265-272) plus the delivered-empty numeric guard, the
+// numeric-duration guard and the comma-split slice hook (so a single env var can express
+// a []string). koanf fills in
 // Result and TagName at unmarshal time.
 func buildDecoderConfig() *mapstructure.DecoderConfig {
 	return &mapstructure.DecoderConfig{
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
+			configdecode.EmptyStringToNumericGuardHookFunc(),
 			configdecode.NumericToDurationGuardHookFunc(),
 			stringToTrimmedSliceHookFunc(","),
 			mapstructure.StringToTimeDurationHookFunc(),
@@ -298,14 +300,16 @@ func buildDecoderConfig() *mapstructure.DecoderConfig {
 
 // unmarshalDecoderConfig is the decoder for the public Config.Unmarshal. It mirrors koanf's
 // default Unmarshal decoder (StringToTimeDurationHookFunc + text-unmarshaler + WeaklyTypedInput)
-// plus only the numeric-duration guard — deliberately WITHOUT the comma-split slice hook, so
+// plus the delivered-empty numeric guard and the numeric-duration guard — deliberately
+// WITHOUT the comma-split slice hook, so
 // string -> []string keeps koanf's default single-element wrap on this public seam instead of
 // silently comma-splitting. (koanf's default uses its own unexported textUnmarshalerHookFunc;
 // the exported mapstructure.TextUnmarshallerHookFunc is the closest public equivalent and
 // differs only for custom string types no framework config field uses.)
 func unmarshalDecoderConfig() *mapstructure.DecoderConfig {
 	return &mapstructure.DecoderConfig{
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
+			configdecode.EmptyStringToNumericGuardHookFunc(),
 			configdecode.NumericToDurationGuardHookFunc(),
 			mapstructure.StringToTimeDurationHookFunc(),
 			mapstructure.TextUnmarshallerHookFunc(),

config/config_test.go

@@ -185,6 +185,74 @@ func TestConfigPerTenantJobKeys(t *testing.T) {
 	})
 }
 
+// TestLoadRejectsEmptyNumericEnv pins the delivered-empty rule for numeric keys: a
+// set-but-empty variable used to decode as a legal 0 (defeating ADR-065's tri-state and
+// silently zeroing byte limits), and now fails Load naming the key.
+func TestLoadRejectsEmptyNumericEnv(t *testing.T) {
+	tests := []struct {
+		name    string
+		envVar  string
+		wantKey string
+	}{
+		{name: "keystore_secretminlength", envVar: "KEYSTORE_SECRETMINLENGTH", wantKey: "keystore.secretminlength"},
+		{name: "server_bodylimit", envVar: "SERVER_BODYLIMIT", wantKey: "server.bodylimit"},
+		{name: "server_port", envVar: "SERVER_PORT", wantKey: "server.port"},
+		// database.port is an ADR-051 identity key AND numeric, so the numeric guard
+		// reaches it first: it now fails at decode rather than with the identity error.
+		{name: "database_port_changes_error_class", envVar: "DATABASE_PORT", wantKey: "database.port"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			clearEnvironmentVariables()
+			t.Setenv(tt.envVar, "")
+
+			_, err := Load()
+
+			require.Error(t, err)
+			assert.ErrorContains(t, err, tt.wantKey)
+			assert.ErrorContains(t, err, "delivered empty")
+		})
+	}
+}
+
+// TestLoadEmptyDurationEnvKeepsItsOwnError pins the guard's one exemption: time.Duration
+// targets fall through to the duration parser, so an empty duration still fails with the
+// parse error rather than the delivered-empty one. Both are loud; this pins which.
+func TestLoadEmptyDurationEnvKeepsItsOwnError(t *testing.T) {
+	clearEnvironmentVariables()
+	t.Setenv("SERVER_TIMEOUT_READ", "")
+
+	_, err := Load()
+
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "invalid duration")
+	assert.NotContains(t, err.Error(), "delivered empty",
+		"the duration parser owns this target; guarding it here would only change the message")
+}
+
+// TestLoadEmptyNumericYAMLStringRejected covers the same rule arriving through YAML: an
+// empty string takes the identical decode path an empty env var does.
+func TestLoadEmptyNumericYAMLStringRejected(t *testing.T) {
+	_, err := loadDeliveredEmptyFixture(t, "keystore:\n  secretminlength: \"\"\n", nil)
+
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "secretminlength")
+	assert.ErrorContains(t, err, "delivered empty")
+}
+
+// TestLoadYAMLNullNumericKeepsTodaysDecode pins the boundary the guard deliberately does
+// NOT cover: a YAML null is different plumbing — koanf delivers a nil value, not the ""
+// the guard judges — so a null pointer key still decodes as absent and takes its default.
+// Documented in ADR-074; this test exists so the boundary cannot drift unnoticed.
+func TestLoadYAMLNullNumericKeepsTodaysDecode(t *testing.T) {
+	cfg, err := loadDeliveredEmptyFixture(t, "keystore:\n  secretminlength:\n", nil)
+
+	require.NoError(t, err)
+	require.NotNil(t, cfg.KeyStore.SecretMinLength)
+	assert.Equal(t, 32, *cfg.KeyStore.SecretMinLength, "a null key is absence, so the floor still applies")
+}
+
 func TestLoadMultiElementStringSliceEnv(t *testing.T) {
 	clearEnvironmentVariables()
 	t.Setenv("SCHEDULER_SECURITY_CIDRALLOWLIST", "10.0.0.0/8,192.168.0.0/16")