Skip to content

feat: Modular v2 enhancements — 12 gaps + modernization#194

Closed
intel352 wants to merge 40 commits intoCrisisTextLine:mainfrom
GoCodeAlone:feat/reimplementation
Closed

feat: Modular v2 enhancements — 12 gaps + modernization#194
intel352 wants to merge 40 commits intoCrisisTextLine:mainfrom
GoCodeAlone:feat/reimplementation

Conversation

@intel352
Copy link

Summary

Addresses all 12 gaps identified in the Modular framework audit, making it a more complete foundation for the Workflow engine and other consumers. Also modernizes the entire codebase using gopls quickfixes.

Core Lifecycle (Section 1)

  • Config-driven dependency hints: WithModuleDependency(from, to) builder option injects edges into the dependency graph before resolution
  • Drainable interface: PreStop(ctx) called before Stop() for graceful drain of in-flight work, with configurable WithDrainTimeout(d)
  • Application phase tracking: Phase() method returning AppPhase enum (Created → Initializing → Initialized → Starting → Running → Draining → Stopping → Stopped), emits EventTypeAppPhaseChanged CloudEvents
  • Parallel init: WithParallelInit() initializes modules at the same topological depth concurrently via errgroup

Services & Plugins (Section 2)

  • Type-safe service helpers: RegisterTypedService[T] / GetTypedService[T] generic functions with compile-time type safety
  • Service readiness events: OnServiceReady(name, callback) fires immediately if registered or defers until registration
  • Plugin interface: Plugin / PluginWithHooks / PluginWithServices interfaces with WithPlugins(...) builder option

Configuration & Reload (Section 3)

  • ReloadOrchestrator integration: WithDynamicReload() wires the orchestrator into the app lifecycle
  • Config file watcher: modules/configwatcher package using fsnotify with debounced change detection
  • Secret resolution hooks: SecretResolver interface + ExpandSecrets() for ${prefix:path} patterns (handles maps, nested maps, and slices)

Observability (Section 4)

  • Slog adapter: SlogAdapter wrapping *slog.Logger with With() / WithGroup() chaining
  • Module metrics hooks: MetricsProvider interface + CollectAllMetrics(ctx) for vendor-neutral metric collection

Code Quality

  • Thread-safe EnhancedServiceRegistry with sync.RWMutex (safe for parallel init)
  • ErrDynamicReloadNotEnabled sentinel error
  • ConfigWatcher implements Module interface
  • gopls modernization across 63 files: interface{}any, reflect.TypeOfreflect.TypeFor[T], reflect.Ptrreflect.Pointer, manual loops → slices.Contains/maps.Copy, C-style for → range N

Design

See: docs/plans/2026-03-09-modular-v2-enhancements-design.md

Implementation Plan

See: docs/plans/2026-03-09-modular-v2-enhancements.md

Test plan

  • All 12 features have dedicated test files with edge cases
  • go test -count=1 -race ./... passes (root + configwatcher packages)
  • go build ./... compiles clean
  • Pre-existing feeders/yaml.go vet errors are unrelated to this PR

🤖 Generated with Claude Code

Copilot AI and others added 30 commits March 6, 2026 21:31
…ests

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
…ategies

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
… fix MakeJSONResponse status format

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Reset GoCodeAlone/modular to CrisisTextLine/modular main (v1.11.11+16 commits).
Changed all module paths from CrisisTextLine to GoCodeAlone.
Merged CrisisTextLine#192 (composite route strategies).
Added reimplementation plans for previously GoCodeAlone-specific features:
- TenantGuard framework
- Dynamic Reload Manager
- Aggregate Health Service
- BDD/Contract Testing framework

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update all sub-modules to reference GoCodeAlone/modular v1.12.0.
Add replace directive for eventbus mocks resolution.
Fix letsencrypt httpserver dependency version.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The eventbus/v2 go.mod had a require + replace for the v1 eventbus
module path (for mocks). The replace directive doesn't propagate to
consumers, causing 'unknown revision' errors when downstream modules
run go mod tidy.

Fix: change test imports from .../eventbus/mocks to .../eventbus/v2/mocks
(the correct v2 import path) and remove the v1 require/replace.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Analyzed existing codebase against all 4 plans:
- TenantGuard: ~50% exists (context, service, config, decorators)
- Dynamic Reload: ~25% exists (observer, field tracking, config providers)
- Aggregate Health: ~15% exists (reverseproxy health checker)
- BDD/Contract Testing: ~65% exists (121 tests, contract CLI, CI)

Revised checklists to only cover remaining work.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… emission

Implements HealthProvider interface, HealthReport/AggregatedHealth types,
provider adapters (simple, static, composite), and AggregateHealthService
with fan-out evaluation, panic recovery, cache TTL, temporary error
detection, and CloudEvent emission on status changes. 17 tests including
concurrency and race-detector verified.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduces TenantGuard interface and StandardTenantGuard implementation
with strict/lenient/disabled modes, whitelist support, ring buffer
violation tracking, and CloudEvents emission on violations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…and rollback

Add Reloadable interface to module.go for modules that support runtime
config reloading. Implement ReloadOrchestrator with single-flight
execution, exponential backoff circuit breaker, reverse-order rollback
on partial failure, and CloudEvents emission for reload lifecycle.

New files:
- reload.go: ChangeType, ConfigChange, FieldChange, ConfigDiff, ReloadTrigger types
- reload_orchestrator.go: ReloadOrchestrator with queue, circuit breaker, rollback
- reload_test.go: comprehensive tests (ConfigDiff, orchestrator, rollback, circuit breaker, concurrency)

Also fix pre-existing WithLogger name collision in health_service.go -> WithHealthLogger.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace dynamic errors.New() with sentinel errors (err113)
- Add StatusUnknown case to exhaustive switch (exhaustive)
- Derive request context from parent context (contextcheck)
- Wrap interface method error return (wrapcheck)
- Fix gofmt alignment in builder.go, contract_verifier.go, observer.go
- Update letsencrypt go.sum for httpserver@v1.12.0 checksum

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reload orchestrator:
- Sort targets by module name for deterministic reload/rollback order
- Make Stop() idempotent with sync.Once, reject requests after stop
- Move noop check before started event to avoid misleading event stream
- Apply default 30s timeout when ReloadTimeout() returns <= 0
- Add doc comment noting application integration is a follow-up

Health service:
- Switch from *log.Logger to modular.Logger for consistent structured logging
- Return deep copies from Check() to prevent cache mutation by callers
- Use NewCloudEvent helper for proper event ID/specversion
- Prefer StatusUnhealthy over StatusUnknown on tie-breaks

Tenant guard:
- Switch from *log.Logger to modular.Logger for consistent structured logging
- Deep-copy whitelist in constructor, convert to set for O(1) lookups
- Use NewCloudEvent helper for proper event ID/specversion
- Use structured logging (Warn with key-value pairs) instead of Printf
- Wire guard into application service registry via builder

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add ErrReloadStopped for stopped orchestrator (distinct from channel full)
- Guard against nil logger with nopLogger fallback in NewReloadOrchestrator
- Fix BenchmarkReload to call processReload directly instead of queue+drain
- Update rollback test to assert deterministic sorted order

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- reload_orchestrator: add recover guard in RequestReload to prevent
  panic on send-to-closed-channel race with Stop()
- builder: use app.RegisterService() instead of direct SvcRegistry map
  mutation for tenant guard registration
- reload_contract_bdd_test: make rollback assertion deterministic now
  that targets are sorted by name

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- reload_orchestrator: extract handleReload() to properly scope context
  lifetime (no defer cancel leak in loop), propagate req.Ctx values and
  cancellation to module Reload calls instead of only copying deadline
- health_service: worstStatus now maps StatusUnknown → StatusUnhealthy
  in output, consistent with documented aggregation behavior
- reload_test: replace all time.Sleep waits with polling waitFor helper
  for deterministic CI-safe synchronization
- reload_contract_bdd_test: replace time.Sleep waits with event/call
  polling helpers (bddWaitForEvent, bddWaitForCalls)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep context chain rooted in parentCtx and apply request deadline via
context.WithDeadline instead of swapping the base context.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- contract_verifier: guard checkReloadIdempotent with goroutine+select
  so a misbehaving module cannot block the verifier indefinitely
- health_service: Check now selects on ctx.Done() when collecting
  provider results, returning ctx.Err() on cancellation/timeout
- tenant_guard: log NotifyObservers errors instead of silently dropping;
  update TenantGuardLenient doc to clarify logging is best-effort
- reload_contract_bdd_test: simulate elapsed backoff by backdating
  lastFailure instead of manually clearing circuit breaker state
- reload_orchestrator: propagate req.Ctx cancellation via background
  goroutine watching req.Ctx.Done(), not just deadline

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add ErrReloadTimeout sentinel error, use in contract verifier
- Wrap ctx.Err() in health_service.Check for wrapcheck compliance

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers 12 gaps identified in framework audit: config-driven deps,
drainable shutdown, phase tracking, parallel init, type-safe services,
service readiness events, plugin interface, reload integration,
config file watcher, secret resolution, slog adapter, metrics hooks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
12 tasks covering all audit gaps: config-driven deps, drainable
shutdown, phase tracking, parallel init, type-safe services,
service readiness, plugin interface, reload integration,
secret resolver, config watcher, slog adapter, metrics hooks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hints

Allow injecting dependency edges into the module dependency graph from
the builder/config level, without requiring modules to implement the
DependencyAware interface. Hints are merged into the graph before
topological sort, enabling correct init ordering and cycle detection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
intel352 and others added 10 commits March 9, 2026 23:24
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ycle

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add sync.RWMutex to EnhancedServiceRegistry for thread-safe concurrent access
- Protect all registry methods (RegisterService, GetService, OnServiceReady, etc.)
- Fire readiness callbacks outside the lock to prevent deadlocks
- Add Init(modular.Application) to ConfigWatcher to satisfy Module interface
- Add ErrDynamicReloadNotEnabled sentinel error
- Add []any slice handling to ExpandSecrets for YAML array configs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Applied gopls modernization across 63 files:
- Replace interface{} with any (Go 1.18+)
- Replace reflect.TypeOf((*T)(nil)).Elem() with reflect.TypeFor[T]() (Go 1.22+)
- Replace reflect.Ptr with reflect.Pointer
- Replace manual loops with slices.Contains and maps.Copy
- Replace C-style for loops with range N syntax (Go 1.22+)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 10, 2026 03:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@intel352 intel352 closed this Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants