refactor: wire alert factory, doctor helpers, and IdGen (Phase 4)#687
Closed
Aaronontheweb wants to merge 2 commits intodevfrom
Closed
refactor: wire alert factory, doctor helpers, and IdGen (Phase 4)#687Aaronontheweb wants to merge 2 commits intodevfrom
Aaronontheweb wants to merge 2 commits intodevfrom
Conversation
…agnostics Additive-only extraction of shared helpers to eliminate duplicated code across pipeline actors, alert emission sites, and doctor checks. No existing behavior changes — all helpers are new code with new tests. New types: - SessionPipelineHandle: composition helper for pipeline lifecycle (init, reinit, dispose) - ExecutionOutputAccumulator: output buffering + notification tracking for execution actors - OperationalAlert.Create(): factory method normalizing alert construction (replaces 16 sites) - IdGen: centralized ID generation (AlertId, ShortId, Suffix, Full) - CrashLogHelper: shared crash-log parsing for doctor checks - DoctorJsonConfigReader.ReadBool/ReadStringArray: shared JSON helpers Test infrastructure: - ScriptedSessionPipeline and FailingSessionPipeline extracted to shared TestHelpers - 22 new unit tests covering all extracted helpers Refs #306
Collaborator
Author
|
Superseded by #686 which now includes the full consolidation. |
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
new OperationalAlert { ... }constructions across 10 production files with calls toOperationalAlert.Create(timeProvider, ...), which auto-generatesAlertIdviaIdGen.AlertId()and setsTimestampfrom the injectedTimeProvider. Also fixes a bug inBinaryUpdateCheckServicewhereAlertIdwas set to a full 32-char GUID instead of the expected 12-char truncation.ReadBoolcopies and 1 privateReadStringArraycopy from doctor check files, replacing all call sites withDoctorJsonConfigReader.ReadBool(...)andDoctorJsonConfigReader.ReadStringArray(...).IsCrashLogStaleand 2 privateTryParseCrashTimestampcopies from doctor check files, replacing all call sites with sharedCrashLogHelpermethods.Guid.NewGuid().ToString("N")[..8]patterns withIdGen.ShortId()and 2Guid.NewGuid().ToString("N")[..6]patterns withIdGen.Suffix().Files changed (22)
OperationalAlert.Create() wiring (10 files, 16 sites):
src/Netclaw.Daemon/Services/DaemonLifecycleNotifier.cs(3 sites)src/Netclaw.Daemon/Mcp/McpClientManager.cs(2 sites)src/Netclaw.Daemon/Mcp/McpOAuthService.cs(2 sites)src/Netclaw.Daemon/Configuration/FailoverChatClient.cs(2 sites)src/Netclaw.Daemon/Configuration/AlertingChatClientDecorator.cs(1 site)src/Netclaw.Daemon/Services/BinaryUpdateCheckService.cs(1 site, also fixes missing [..12] truncation)src/Netclaw.Daemon/Webhooks/WebhookEndpointRouteBuilderExtensions.cs(1 site)src/Netclaw.Daemon/Webhooks/WebhookRouteCatalog.cs(1 site)src/Netclaw.Channels.Slack/SlackChannel.cs(1 site)src/Netclaw.Actors/Reminders/ReminderManagerActor.cs(2 sites)Doctor helper dedup (6 files):
src/Netclaw.Cli/Doctor/SlackAuthDoctorCheck.cs(removed private ReadBool)src/Netclaw.Cli/Doctor/SlackAclDoctorCheck.cs(removed private ReadBool + ReadStringArray)src/Netclaw.Cli/Doctor/TelemetryDoctorCheck.cs(removed private ReadBool)src/Netclaw.Cli/Doctor/DoctorFixService.cs(removed private ReadBool)src/Netclaw.Cli/Doctor/DaemonCrashDoctorCheck.cs(removed IsCrashLogStale + TryParseCrashTimestamp)src/Netclaw.Cli/Doctor/SqliteProvisioningDoctorCheck.cs(removed IsCrashLogStale + TryParseCrashTimestamp)IdGen wiring (6 files, 7 sites):
src/Netclaw.Actors/Channels/ChannelPipeline.cs(ShortId)src/Netclaw.Channels.Slack/SlackConversationActor.cs(ShortId)src/Netclaw.Actors/Sessions/LlmSessionActor.cs(ShortId)src/Netclaw.Cli/Tui/ProviderManagerViewModel.cs(ShortId + Suffix)src/Netclaw.Cli/Tui/ModelManagerViewModel.cs(ShortId)src/Netclaw.Actors/Reminders/ReminderIdGenerator.cs(Suffix)Net impact
Test plan
dotnet buildsucceeds with 0 errors, 0 warningsdotnet testpasses all 2,482 tests (0 failures)