feat: Modular v2 enhancements — 12 gaps + modernization#194
Closed
intel352 wants to merge 40 commits intoCrisisTextLine:mainfrom
Closed
feat: Modular v2 enhancements — 12 gaps + modernization#194intel352 wants to merge 40 commits intoCrisisTextLine:mainfrom
intel352 wants to merge 40 commits intoCrisisTextLine:mainfrom
Conversation
…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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)
WithModuleDependency(from, to)builder option injects edges into the dependency graph before resolutionPreStop(ctx)called beforeStop()for graceful drain of in-flight work, with configurableWithDrainTimeout(d)Phase()method returningAppPhaseenum (Created → Initializing → Initialized → Starting → Running → Draining → Stopping → Stopped), emitsEventTypeAppPhaseChangedCloudEventsWithParallelInit()initializes modules at the same topological depth concurrently via errgroupServices & Plugins (Section 2)
RegisterTypedService[T]/GetTypedService[T]generic functions with compile-time type safetyOnServiceReady(name, callback)fires immediately if registered or defers until registrationPlugin/PluginWithHooks/PluginWithServicesinterfaces withWithPlugins(...)builder optionConfiguration & Reload (Section 3)
WithDynamicReload()wires the orchestrator into the app lifecyclemodules/configwatcherpackage using fsnotify with debounced change detectionSecretResolverinterface +ExpandSecrets()for${prefix:path}patterns (handles maps, nested maps, and slices)Observability (Section 4)
SlogAdapterwrapping*slog.LoggerwithWith()/WithGroup()chainingMetricsProviderinterface +CollectAllMetrics(ctx)for vendor-neutral metric collectionCode Quality
EnhancedServiceRegistrywithsync.RWMutex(safe for parallel init)ErrDynamicReloadNotEnabledsentinel errorModuleinterfaceinterface{}→any,reflect.TypeOf→reflect.TypeFor[T],reflect.Ptr→reflect.Pointer, manual loops →slices.Contains/maps.Copy, C-style for →range NDesign
See:
docs/plans/2026-03-09-modular-v2-enhancements-design.mdImplementation Plan
See:
docs/plans/2026-03-09-modular-v2-enhancements.mdTest plan
go test -count=1 -race ./...passes (root + configwatcher packages)go build ./...compiles cleanfeeders/yaml.govet errors are unrelated to this PR🤖 Generated with Claude Code